From 43833b40bbd1ba73fbfc0f5e6a1b1ecb314e0e90 Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 18 Jun 2025 21:44:20 +0700 Subject: [PATCH 01/18] [ticket/17528] Add test for adding autoincrement column PHPBB-17528 --- .../migration/schema_add_autoincrement.php | 43 +++++++++++++++++++ tests/dbal/migrator_test.php | 21 +++++++++ 2 files changed, 64 insertions(+) create mode 100644 tests/dbal/migration/schema_add_autoincrement.php diff --git a/tests/dbal/migration/schema_add_autoincrement.php b/tests/dbal/migration/schema_add_autoincrement.php new file mode 100644 index 0000000000..4e3ecff183 --- /dev/null +++ b/tests/dbal/migration/schema_add_autoincrement.php @@ -0,0 +1,43 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +class schema_add_autoincrement extends \phpbb\db\migration\migration +{ + function update_schema() + { + return [ + 'add_tables' => [ + $this->table_prefix . 'noid' => [ + 'COLUMNS' => [ + 'text' => ['VCHAR:50', ''], + ], + ], + ], + + 'add_columns' => [ + $this->table_prefix . 'noid' => [ + 'id' => ['UINT:3', null, 'auto_increment'], + ], + ], + ]; + } + + function revert_schema() + { + return [ + 'drop_tables' => [ + $this->table_prefix . 'noid', + ], + ]; + } +} diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php index f4b80fc5e9..284c52a07c 100644 --- a/tests/dbal/migrator_test.php +++ b/tests/dbal/migrator_test.php @@ -25,6 +25,7 @@ require_once __DIR__ . '/migration/fail.php'; require_once __DIR__ . '/migration/installed.php'; require_once __DIR__ . '/migration/schema.php'; require_once __DIR__ . '/migration/schema_index.php'; +require_once __DIR__ . '/migration/schema_add_autoincrement.php'; class phpbb_dbal_migrator_test extends phpbb_database_test_case { @@ -502,4 +503,24 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case } } } + + public function test_add_autoincrement_column() + { + $this->migrator->set_migrations(['schema_add_autoincrement']); + + while (!$this->migrator->finished()) + { + $this->migrator->update(); + } + + $this->assertTrue($this->db_tools->sql_table_exists('phpbb_noid')); + $this->assertTrue($this->db_tools->sql_column_exists('phpbb_noid', 'id')); + + while ($this->migrator->migration_state('schema_add_autoincrement')) + { + $this->migrator->revert('schema_add_autoincrement'); + } + + $this->assertFalse($this->db_tools->sql_table_exists('phpbb_noid')); + } } From 9cccf2631181eafd38fd0d10d0275c4c630a82ff Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 19 Jun 2025 11:54:17 +0700 Subject: [PATCH 02/18] [ticket/17528] Fix phpBB PostgreSQL platform [ci skip] PHPBB-17528 --- .../phpbb/db/doctrine/postgresql_platform.php | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/phpBB/phpbb/db/doctrine/postgresql_platform.php b/phpBB/phpbb/db/doctrine/postgresql_platform.php index ea637b4203..28f846de03 100644 --- a/phpBB/phpbb/db/doctrine/postgresql_platform.php +++ b/phpBB/phpbb/db/doctrine/postgresql_platform.php @@ -78,6 +78,36 @@ class postgresql_platform extends PostgreSQL94Platform return AbstractPlatform::getDefaultValueDeclarationSQL($column); } + /** + * {@inheritDoc} + */ + public function getAlterTableSQL(TableDiff $diff) + { + $sql = parent::getAlterTableSQL($diff); + $table_name = $diff->getOldTable()->getName(); + $columns = array_merge($diff->getAddedColumns(), $diff->getModifiedColumns()); + $post_sql = $sequence_sql = []; + + foreach ($columns as $column) + { + $column_name = $column->getName(); + if (!empty($column->getAutoincrement())) + { + $sequence = new Sequence($this->getIdentitySequenceName($table_name, $column_name)); + $sequence_sql[] = $this->getCreateSequenceSQL($sequence); + $post_sql[] = 'ALTER SEQUENCE '.$sequence->getName().' OWNED BY ' . $table_name . '.' . $column_name; + } + } + $sql = array_merge($sequence_sql, $sql, $post_sql); + + foreach ($sql as $i => $query) + { + $sql[$i] = str_replace('{{placeholder_sequence}}', "nextval('{$table_name}_seq')", $query); + } + + return $sql; + } + /** * {@inheritDoc} */ From 98c6a8843897a923418b090b03ef4234be17e796 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 20 Jun 2025 14:46:39 +0700 Subject: [PATCH 03/18] [ticket/17528] Fix MySQL platform PHPBB-17528 --- .../doctrine/connection_parameter_factory.php | 1 + phpBB/phpbb/db/doctrine/mysql_platform.php | 62 +++++++++++++++++++ .../phpbb/db/doctrine/postgresql_platform.php | 1 + 3 files changed, 64 insertions(+) create mode 100644 phpBB/phpbb/db/doctrine/mysql_platform.php diff --git a/phpBB/phpbb/db/doctrine/connection_parameter_factory.php b/phpBB/phpbb/db/doctrine/connection_parameter_factory.php index 740c4b82b7..90e41061fc 100644 --- a/phpBB/phpbb/db/doctrine/connection_parameter_factory.php +++ b/phpBB/phpbb/db/doctrine/connection_parameter_factory.php @@ -149,6 +149,7 @@ class connection_parameter_factory $enrichment_tags = [ 'pdo_mysql' => [ 'charset' => 'UTF8', + 'platform' => new mysql_platform(), ], 'oci8' => [ 'charset' => 'UTF8', diff --git a/phpBB/phpbb/db/doctrine/mysql_platform.php b/phpBB/phpbb/db/doctrine/mysql_platform.php new file mode 100644 index 0000000000..cd368163dd --- /dev/null +++ b/phpBB/phpbb/db/doctrine/mysql_platform.php @@ -0,0 +1,62 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\db\doctrine; + +use Doctrine\DBAL\Platforms\AbstractMySQLPlatform; +use Doctrine\DBAL\Schema\TableDiff; + +/** + * MySQL specific schema handling. + * + * While adding auto_increment column to MySQL, it must be indexed. + * If it's indexed as primary key, it ahould be declared as NOT NULL + * because MySQL primary key columns cannot be NULL. + */ +class mysql_platform extends AbstractMySQLPlatform +{ + /** + * {@inheritDoc} + */ + public function getAlterTableSQL(TableDiff $diff) + { + $sql = parent::getAlterTableSQL($diff); + $table = $diff->getOldTable(); + $columns = $diff->getAddedColumns(); + + foreach ($columns as $column) + { + $column_name = $column->getName(); + if (!empty($column->getAutoincrement()) && $table) + { + foreach ($sql as $i => $query) + { + if (stripos($query, "add $column_name")) + { + if (!$table->getPrimaryKey()) + { + $sql[$i] = str_replace(' DEFAULT NULL', '', $sql[$i]); + $sql[$i] .= ' PRIMARY KEY'; + } + else + { + $sql[$i] .= ", ADD KEY ($column_name)"; + } + } + } + } + } + + return $sql; + } +} diff --git a/phpBB/phpbb/db/doctrine/postgresql_platform.php b/phpBB/phpbb/db/doctrine/postgresql_platform.php index 28f846de03..754ea47a99 100644 --- a/phpBB/phpbb/db/doctrine/postgresql_platform.php +++ b/phpBB/phpbb/db/doctrine/postgresql_platform.php @@ -18,6 +18,7 @@ use Doctrine\DBAL\Platforms\PostgreSQL94Platform; use Doctrine\DBAL\Schema\Index; use Doctrine\DBAL\Schema\Sequence; use Doctrine\DBAL\Schema\Table; +use Doctrine\DBAL\Schema\TableDiff; use Doctrine\DBAL\Types\BigIntType; use Doctrine\DBAL\Types\IntegerType; use Doctrine\DBAL\Types\SmallIntType; From 83f1ed8de41e2769365d2f2932c459a0c82db237 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 20 Jun 2025 15:26:57 +0700 Subject: [PATCH 04/18] [ticket/17528] Fix PostgreSQL platform issues PHPBB-17528 --- phpBB/phpbb/db/doctrine/postgresql_platform.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/phpBB/phpbb/db/doctrine/postgresql_platform.php b/phpBB/phpbb/db/doctrine/postgresql_platform.php index 754ea47a99..2fa226b3ab 100644 --- a/phpBB/phpbb/db/doctrine/postgresql_platform.php +++ b/phpBB/phpbb/db/doctrine/postgresql_platform.php @@ -79,14 +79,14 @@ class postgresql_platform extends PostgreSQL94Platform return AbstractPlatform::getDefaultValueDeclarationSQL($column); } - /** - * {@inheritDoc} - */ - public function getAlterTableSQL(TableDiff $diff) - { + /** + * {@inheritDoc} + */ + public function getAlterTableSQL(TableDiff $diff) + { $sql = parent::getAlterTableSQL($diff); $table_name = $diff->getOldTable()->getName(); - $columns = array_merge($diff->getAddedColumns(), $diff->getModifiedColumns()); + $columns = $diff->getAddedColumns(); $post_sql = $sequence_sql = []; foreach ($columns as $column) From 53360e0c17ebcb3557a37891604c749722c30663 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 5 Jul 2025 13:26:53 +0700 Subject: [PATCH 05/18] [ticket/17528] Fix typo PHPBB-17528 --- phpBB/phpbb/db/doctrine/mysql_platform.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/db/doctrine/mysql_platform.php b/phpBB/phpbb/db/doctrine/mysql_platform.php index cd368163dd..8a2090b6d9 100644 --- a/phpBB/phpbb/db/doctrine/mysql_platform.php +++ b/phpBB/phpbb/db/doctrine/mysql_platform.php @@ -20,7 +20,7 @@ use Doctrine\DBAL\Schema\TableDiff; * MySQL specific schema handling. * * While adding auto_increment column to MySQL, it must be indexed. - * If it's indexed as primary key, it ahould be declared as NOT NULL + * If it's indexed as primary key, it should be declared as NOT NULL * because MySQL primary key columns cannot be NULL. */ class mysql_platform extends AbstractMySQLPlatform From 4f89e2b97d840b06976584247b5285ed54a918c3 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 5 Jul 2025 09:36:53 +0200 Subject: [PATCH 06/18] [ticket/17528] Fix migration issue using postgres PHPBB-17528 --- phpBB/phpbb/db/migration/data/v400/remove_jabber.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v400/remove_jabber.php b/phpBB/phpbb/db/migration/data/v400/remove_jabber.php index 12759f5f47..34c4c6a355 100644 --- a/phpBB/phpbb/db/migration/data/v400/remove_jabber.php +++ b/phpBB/phpbb/db/migration/data/v400/remove_jabber.php @@ -101,15 +101,11 @@ class remove_jabber extends migration ]; } - public function move_jabber_to_email_notifications(int|null $start) + public function move_jabber_to_email_notifications() { - $limit = 1000; - $sql = 'UPDATE ' . $this->tables['user_notifications'] . ' SET ' . $this->db->sql_build_array('UPDATE', ['method' => 'notification.method.email']) . " WHERE method = 'notification.method.jabber'"; - $this->db->sql_query_limit($sql, $limit, $start ?: 0); - - return $this->db->sql_affectedrows() < $limit ? true : $start + $limit; + $this->db->sql_query($sql); } } From e4cd3ac744338a24746ba3e61ae4317ed38f34e8 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 5 Jul 2025 22:23:26 +0700 Subject: [PATCH 07/18] [ticket/17507] General SQL error on installing remove_jabber.php migration The issue affects probably any DBMS excluding MySQL/MariaDB. PHPBB-17507 --- .../db/migration/data/v400/remove_jabber.php | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v400/remove_jabber.php b/phpBB/phpbb/db/migration/data/v400/remove_jabber.php index 34c4c6a355..82b25b819f 100644 --- a/phpBB/phpbb/db/migration/data/v400/remove_jabber.php +++ b/phpBB/phpbb/db/migration/data/v400/remove_jabber.php @@ -34,6 +34,11 @@ class remove_jabber extends migration $this->table_prefix . 'users' => [ 'user_jabber', ], + ], + 'add_columns' => [ + $this->table_prefix . 'user_notifications' => [ + 'id' => ['ULINT', null, 'auto_increment'], + ], ] ]; } @@ -45,7 +50,12 @@ class remove_jabber extends migration $this->table_prefix . 'users' => [ 'user_jabber' => ['VCHAR_UNI', ''], ], - ] + ], + 'drop_columns' => [ + $this->table_prefix . 'user_notifications' => [ + 'id', + ], + ], ]; } @@ -101,11 +111,26 @@ class remove_jabber extends migration ]; } - public function move_jabber_to_email_notifications() + public function move_jabber_to_email_notifications(int|null $start) { - $sql = 'UPDATE ' . $this->tables['user_notifications'] . ' - SET ' . $this->db->sql_build_array('UPDATE', ['method' => 'notification.method.email']) . " - WHERE method = 'notification.method.jabber'"; - $this->db->sql_query($sql); + $limit = 1000; + + $sql = 'SELECT id FROM ' . $this->tables['user_notifications'] . " + WHERE method = 'notification.method.jabber' + ORDER BY id ASC"; + $result = $this->db->sql_query_limit($sql, $limit, $start ?: 0); + $rowset = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); + $ids_array = array_column($rowset, 'id'); + + if (count($ids_array)) + { + $sql = 'UPDATE ' . $this->tables['user_notifications'] . ' + SET ' . $this->db->sql_build_array('UPDATE', ['method' => 'notification.method.email']) . ' + WHERE ' . $this->db->sql_in_set('id', $ids_array); + $this->db->sql_query($sql); + } + + return count($ids_array) < $limit ? true : $start + $limit; } } From 38ee655b2ae2cec9503151b0c08e03281d50d743 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 5 Jul 2025 23:05:13 +0700 Subject: [PATCH 08/18] [ticket/17507] Add id as primary key, fix schema_create_primary_key() method PHPBB-17507 --- phpBB/phpbb/db/migration/data/v400/remove_jabber.php | 5 ++++- phpBB/phpbb/db/tools/doctrine.php | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v400/remove_jabber.php b/phpBB/phpbb/db/migration/data/v400/remove_jabber.php index 82b25b819f..17bf49d753 100644 --- a/phpBB/phpbb/db/migration/data/v400/remove_jabber.php +++ b/phpBB/phpbb/db/migration/data/v400/remove_jabber.php @@ -39,7 +39,10 @@ class remove_jabber extends migration $this->table_prefix . 'user_notifications' => [ 'id' => ['ULINT', null, 'auto_increment'], ], - ] + ], + 'add_primary_keys' => [ + $this->table_prefix . 'user_notifications' => ['id'], + ], ]; } diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index 043252b4c5..2675c59e30 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -993,13 +993,14 @@ class doctrine implements tools_interface * @param $column * @param Schema $schema * @param string $table_name + * @param array|string $column_name * @param bool $safe_check * * @throws SchemaException */ - protected function schema_create_primary_key(Schema $schema, $column, string $table_name, bool $safe_check = false): void + protected function schema_create_primary_key(Schema $schema, string $table_name, array|string $column_name, bool $safe_check = false): void { - $columns = (is_array($column)) ? $column : [$column]; + $columns = (is_array($column_name)) ? $column_name : [$column_name]; $table = $schema->getTable($table_name); $table->dropPrimaryKey(); $table->setPrimaryKey($columns); From 1d34b4f06a5012f2d6d730653fc07b313239d1c8 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 5 Jul 2025 23:20:22 +0700 Subject: [PATCH 09/18] [ticket/17507] Fix 'add_primary_keys' db tools option for schema generator PHPBB-17507 --- phpBB/phpbb/db/migration/schema_generator.php | 1 + phpBB/phpbb/db/tools/doctrine.php | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/migration/schema_generator.php b/phpBB/phpbb/db/migration/schema_generator.php index 90cfddbe5a..1153f4c67e 100644 --- a/phpBB/phpbb/db/migration/schema_generator.php +++ b/phpBB/phpbb/db/migration/schema_generator.php @@ -185,6 +185,7 @@ class schema_generator 'drop_columns' => 'COLUMNS', 'change_columns' => 'COLUMNS', 'add_index' => 'KEYS', + 'add_primary_keys' => 'PRIMARY_KEY', 'add_unique_index' => 'KEYS', 'drop_keys' => 'KEYS', 'rename_index' => 'KEYS', diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index 2675c59e30..2d8908c9bc 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -398,7 +398,7 @@ class doctrine implements tools_interface return $this->alter_schema( function (Schema $schema) use ($table_name, $column): void { - $this->schema_create_primary_key($schema, $column, $table_name); + $this->schema_create_primary_key($schema, $table_name, $column); } ); } @@ -990,7 +990,6 @@ class doctrine implements tools_interface /** * Creates primary key for a table * - * @param $column * @param Schema $schema * @param string $table_name * @param array|string $column_name From 4b420e295806e36f30840edb8e591435572c323d Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 5 Jul 2025 23:43:09 +0700 Subject: [PATCH 10/18] [ticket/17507] Fix notification related tests PHPBB-17507 --- .../fixtures/email_notification.type.post.xml | 15 +++++++++++++++ .../submit_post_notification.type.bookmark.xml | 7 +++++++ .../submit_post_notification.type.forum.xml | 15 +++++++++++++++ .../submit_post_notification.type.mention.xml | 7 +++++++ .../submit_post_notification.type.post.xml | 15 +++++++++++++++ ...ubmit_post_notification.type.post_in_queue.xml | 8 ++++++++ .../submit_post_notification.type.quote.xml | 6 ++++++ .../submit_post_notification.type.topic.xml | 5 +++++ .../fixtures/webpush_notification.type.post.xml | 15 +++++++++++++++ 9 files changed, 93 insertions(+) diff --git a/tests/notification/fixtures/email_notification.type.post.xml b/tests/notification/fixtures/email_notification.type.post.xml index 6d064141b7..7a4a7191eb 100644 --- a/tests/notification/fixtures/email_notification.type.post.xml +++ b/tests/notification/fixtures/email_notification.type.post.xml @@ -164,12 +164,14 @@ + iditem_typeitem_iduser_idmethodnotify + 1 notification.type.post 0 2 @@ -177,6 +179,7 @@ 1 + 2 notification.type.post 0 3 @@ -184,6 +187,7 @@ 1 + 3 notification.type.post 0 4 @@ -191,6 +195,7 @@ 1 + 4 notification.type.post 0 5 @@ -198,6 +203,7 @@ 1 + 5 notification.type.post 0 6 @@ -205,6 +211,7 @@ 1 + 6 notification.type.post 0 7 @@ -212,6 +219,7 @@ 1 + 7 notification.type.post 0 8 @@ -219,6 +227,7 @@ 1 + 8 notification.type.forum 0 2 @@ -226,6 +235,7 @@ 1 + 9 notification.type.forum 0 3 @@ -233,6 +243,7 @@ 1 + 10 notification.type.forum 0 4 @@ -240,6 +251,7 @@ 1 + 11 notification.type.forum 0 5 @@ -247,6 +259,7 @@ 1 + 12 notification.type.forum 0 6 @@ -254,6 +267,7 @@ 1 + 13 notification.type.forum 0 7 @@ -261,6 +275,7 @@ 1 + 14 notification.type.forum 0 8 diff --git a/tests/notification/fixtures/submit_post_notification.type.bookmark.xml b/tests/notification/fixtures/submit_post_notification.type.bookmark.xml index b6163e9ed0..1e6b6a19ca 100644 --- a/tests/notification/fixtures/submit_post_notification.type.bookmark.xml +++ b/tests/notification/fixtures/submit_post_notification.type.bookmark.xml @@ -119,12 +119,14 @@
+ iditem_typeitem_iduser_idmethodnotify + 1 notification.type.bookmark 0 2 @@ -132,6 +134,7 @@ 1 + 2 notification.type.bookmark 0 3 @@ -139,6 +142,7 @@ 1 + 3 notification.type.bookmark 0 4 @@ -146,6 +150,7 @@ 1 + 4 notification.type.bookmark 0 5 @@ -153,6 +158,7 @@ 1 + 5 notification.type.bookmark 0 6 @@ -160,6 +166,7 @@ 0 + 6 notification.type.bookmark 0 3 diff --git a/tests/notification/fixtures/submit_post_notification.type.forum.xml b/tests/notification/fixtures/submit_post_notification.type.forum.xml index b8df34bedc..c12c878604 100644 --- a/tests/notification/fixtures/submit_post_notification.type.forum.xml +++ b/tests/notification/fixtures/submit_post_notification.type.forum.xml @@ -155,12 +155,14 @@
+ iditem_typeitem_iduser_idmethodnotify + 1 notification.type.post 0 2 @@ -168,6 +170,7 @@ 1 + 2 notification.type.post 0 3 @@ -175,6 +178,7 @@ 1 + 3 notification.type.post 0 4 @@ -182,6 +186,7 @@ 1 + 4 notification.type.post 0 5 @@ -189,6 +194,7 @@ 1 + 5 notification.type.post 0 6 @@ -196,6 +202,7 @@ 1 + 6 notification.type.post 0 7 @@ -203,6 +210,7 @@ 1 + 7 notification.type.post 0 8 @@ -210,6 +218,7 @@ 1 + 8 notification.type.forum 0 2 @@ -217,6 +226,7 @@ 1 + 9 notification.type.forum 0 3 @@ -224,6 +234,7 @@ 1 + 10 notification.type.forum 0 4 @@ -231,6 +242,7 @@ 1 + 11 notification.type.forum 0 5 @@ -238,6 +250,7 @@ 1 + 12 notification.type.forum 0 6 @@ -245,6 +258,7 @@ 1 + 13 notification.type.forum 0 7 @@ -252,6 +266,7 @@ 1 + 14 notification.type.forum 0 8 diff --git a/tests/notification/fixtures/submit_post_notification.type.mention.xml b/tests/notification/fixtures/submit_post_notification.type.mention.xml index 86ae1fd037..7faf22c401 100644 --- a/tests/notification/fixtures/submit_post_notification.type.mention.xml +++ b/tests/notification/fixtures/submit_post_notification.type.mention.xml @@ -136,12 +136,14 @@
+ iditem_typeitem_iduser_idmethodnotify + 1 notification.type.mention 0 2 @@ -149,6 +151,7 @@ 1 + 2 notification.type.mention 0 3 @@ -156,6 +159,7 @@ 1 + 3 notification.type.mention 0 4 @@ -163,6 +167,7 @@ 1 + 4 notification.type.mention 0 5 @@ -170,6 +175,7 @@ 1 + 5 notification.type.mention 0 6 @@ -177,6 +183,7 @@ 0 + 6 notification.type.mention 0 8 diff --git a/tests/notification/fixtures/submit_post_notification.type.post.xml b/tests/notification/fixtures/submit_post_notification.type.post.xml index 75bfb03e89..7d760cd1eb 100644 --- a/tests/notification/fixtures/submit_post_notification.type.post.xml +++ b/tests/notification/fixtures/submit_post_notification.type.post.xml @@ -155,12 +155,14 @@
+ iditem_typeitem_iduser_idmethodnotify + 1 notification.type.post 0 2 @@ -168,6 +170,7 @@ 1 + 2 notification.type.post 0 3 @@ -175,6 +178,7 @@ 1 + 3 notification.type.post 0 4 @@ -182,6 +186,7 @@ 1 + 4 notification.type.post 0 5 @@ -189,6 +194,7 @@ 1 + 5 notification.type.post 0 6 @@ -196,6 +202,7 @@ 1 + 6 notification.type.post 0 7 @@ -203,6 +210,7 @@ 1 + 7 notification.type.post 0 8 @@ -210,6 +218,7 @@ 1 + 8 notification.type.forum 0 2 @@ -217,6 +226,7 @@ 1 + 9 notification.type.forum 0 3 @@ -224,6 +234,7 @@ 1 + 10 notification.type.forum 0 4 @@ -231,6 +242,7 @@ 1 + 11 notification.type.forum 0 5 @@ -238,6 +250,7 @@ 1 + 12 notification.type.forum 0 6 @@ -245,6 +258,7 @@ 1 + 13 notification.type.forum 0 7 @@ -252,6 +266,7 @@ 1 + 14 notification.type.forum 0 8 diff --git a/tests/notification/fixtures/submit_post_notification.type.post_in_queue.xml b/tests/notification/fixtures/submit_post_notification.type.post_in_queue.xml index 12e73b0ff2..fe0a99bdc5 100644 --- a/tests/notification/fixtures/submit_post_notification.type.post_in_queue.xml +++ b/tests/notification/fixtures/submit_post_notification.type.post_in_queue.xml @@ -103,12 +103,14 @@
+ iditem_typeitem_iduser_idmethodnotify + 1 notification.type.needs_approval 0 2 @@ -116,6 +118,7 @@ 1 + 2 notification.type.needs_approval 0 3 @@ -123,6 +126,7 @@ 1 + 3 notification.type.needs_approval 0 4 @@ -130,6 +134,7 @@ 1 + 4 notification.type.needs_approval 0 5 @@ -137,6 +142,7 @@ 1 + 5 notification.type.needs_approval 0 6 @@ -144,6 +150,7 @@ 1 + 6 notification.type.needs_approval 0 7 @@ -151,6 +158,7 @@ 0 + 7 notification.type.needs_approval 0 9 diff --git a/tests/notification/fixtures/submit_post_notification.type.quote.xml b/tests/notification/fixtures/submit_post_notification.type.quote.xml index 9f4ba91475..8fe85d0db3 100644 --- a/tests/notification/fixtures/submit_post_notification.type.quote.xml +++ b/tests/notification/fixtures/submit_post_notification.type.quote.xml @@ -91,12 +91,14 @@
+ iditem_typeitem_iduser_idmethodnotify + 1 notification.type.quote 0 2 @@ -104,6 +106,7 @@ 1 + 2 notification.type.quote 0 3 @@ -111,6 +114,7 @@ 1 + 3 notification.type.quote 0 4 @@ -118,6 +122,7 @@ 1 + 4 notification.type.quote 0 5 @@ -125,6 +130,7 @@ 1 + 5 notification.type.quote 0 6 diff --git a/tests/notification/fixtures/submit_post_notification.type.topic.xml b/tests/notification/fixtures/submit_post_notification.type.topic.xml index 1f96ed2ee7..eebab64e6b 100644 --- a/tests/notification/fixtures/submit_post_notification.type.topic.xml +++ b/tests/notification/fixtures/submit_post_notification.type.topic.xml @@ -99,12 +99,14 @@
+ iditem_typeitem_iduser_idmethodnotify + 1 notification.type.topic 0 2 @@ -112,6 +114,7 @@ 1 + 2 notification.type.topic 0 6 @@ -119,6 +122,7 @@ 1 + 3 notification.type.topic 0 7 @@ -126,6 +130,7 @@ 1 + 4 notification.type.topic 0 8 diff --git a/tests/notification/fixtures/webpush_notification.type.post.xml b/tests/notification/fixtures/webpush_notification.type.post.xml index d59d5b92c4..89c5354030 100644 --- a/tests/notification/fixtures/webpush_notification.type.post.xml +++ b/tests/notification/fixtures/webpush_notification.type.post.xml @@ -185,12 +185,14 @@
+ iditem_typeitem_iduser_idmethodnotify + 1 notification.type.post 0 2 @@ -198,6 +200,7 @@ 1 + 2 notification.type.post 0 3 @@ -205,6 +208,7 @@ 1 + 3 notification.type.post 0 4 @@ -212,6 +216,7 @@ 1 + 4 notification.type.post 0 5 @@ -219,6 +224,7 @@ 1 + 5 notification.type.post 0 6 @@ -226,6 +232,7 @@ 1 + 6 notification.type.post 0 7 @@ -233,6 +240,7 @@ 1 + 7 notification.type.post 0 8 @@ -240,6 +248,7 @@ 1 + 8 notification.type.forum 0 2 @@ -247,6 +256,7 @@ 1 + 9 notification.type.forum 0 3 @@ -254,6 +264,7 @@ 1 + 10 notification.type.forum 0 4 @@ -261,6 +272,7 @@ 1 + 11 notification.type.forum 0 5 @@ -268,6 +280,7 @@ 1 + 12 notification.type.forum 0 6 @@ -275,6 +288,7 @@ 1 + 13 notification.type.forum 0 7 @@ -282,6 +296,7 @@ 1 + 14 notification.type.forum 0 8 From 59ef2c8fce0be3cc2f0ce1caf8a514725d03c790 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 6 Jul 2025 14:53:13 +0700 Subject: [PATCH 11/18] [ticket/17507] Add remove_jabber migration test PHPBB-17507 --- .../fixtures/migration_remove_jabber.xml | 284 ++++++++++++++++++ .../remove_jabber_migration_test.php | 184 ++++++++++++ 2 files changed, 468 insertions(+) create mode 100644 tests/migrations/fixtures/migration_remove_jabber.xml create mode 100644 tests/migrations/remove_jabber_migration_test.php diff --git a/tests/migrations/fixtures/migration_remove_jabber.xml b/tests/migrations/fixtures/migration_remove_jabber.xml new file mode 100644 index 0000000000..452c7fb40b --- /dev/null +++ b/tests/migrations/fixtures/migration_remove_jabber.xml @@ -0,0 +1,284 @@ + + +
+ migration_name + migration_depends_on + migration_schema_done + migration_data_done + migration_data_state + migration_start_time + migration_end_time +
+ + config_name + config_value + + jab_enable + 1 + + + jab_host + + + + jab_package_size + 0 + + + jab_password + + + + jab_port + + + + jab_use_ssl + + + + jab_username + user + + + jab_verify_peer + 0 + + + jab_verify_peer_name + 0 + + + jab_allow_self_signed + 0 + +
+ + module_id + module_enabled + module_display + module_basename + module_class + parent_id + left_id + right_id + module_langname + module_mode + module_auth + + 1 + 1 + 1 + + acp + 0 + 1 + 76 + ACP_CAT_GENERAL + + + + + 4 + 1 + 1 + + acp + 0 + 48 + 59 + ACP_CLIENT_COMMUNICATION + + + + + 75 + 1 + 1 + acp_jabber + acp + 4 + 53 + 54 + ACP_JABBER_SETTINGS + settings + acl_a_jabber + +
+ + auth_option_id + auth_option + is_global + is_local + founder_only + + 70 + a_jabber + 1 + 0 + 0 + + + 121 + u_sendim + 1 + 0 + 0 + +
+ + group_id + forum_id + auth_option_id + auth_role_id + auth_setting +
+ + role_id + auth_option_id + auth_setting + + 4 + 70 + 1 + + + 5 + 121 + 1 + + + 6 + 121 + 1 + +
+ + user_id + forum_id + auth_option_id + auth_role_id + auth_setting +
+ + id + item_type + item_id + user_id + method + notify + + 1 + notification.type.post + 0 + 2 + notification.method.jabber + 1 + + + 2 + notification.type.post + 0 + 3 + notification.method.jabber + 1 + + + 3 + notification.type.post + 0 + 4 + notification.method.jabber + 1 + + + 4 + notification.type.post + 0 + 5 + notification.method.jabber + 1 + + + 5 + notification.type.post + 0 + 6 + notification.method.jabber + 1 + + + 6 + notification.type.post + 0 + 7 + notification.method.jabber + 1 + + + 7 + notification.type.post + 0 + 8 + notification.method.jabber + 1 + + + 8 + notification.type.forum + 0 + 2 + notification.method.jabber + 1 + + + 9 + notification.type.forum + 0 + 3 + notification.method.jabber + 1 + + + 10 + notification.type.forum + 0 + 4 + notification.method.jabber + 1 + + + 11 + notification.type.forum + 0 + 5 + notification.method.jabber + 1 + + + 12 + notification.type.forum + 0 + 6 + notification.method.jabber + 1 + + + 13 + notification.type.forum + 0 + 7 + notification.method.jabber + 1 + + + 14 + notification.type.forum + 0 + 8 + notification.method.jabber + 1 + +
+ + diff --git a/tests/migrations/remove_jabber_migration_test.php b/tests/migrations/remove_jabber_migration_test.php new file mode 100644 index 0000000000..c18b6ca754 --- /dev/null +++ b/tests/migrations/remove_jabber_migration_test.php @@ -0,0 +1,184 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +class phpbb_migrations_remove_jabber_migration_test extends phpbb_database_test_case +{ + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + + /** @var \Doctrine\DBAL\Connection */ + protected $doctrine_db; + + /** @var \phpbb\db\tools\tools_interface */ + protected $db_tools; + + /** @var \phpbb\db\migrator */ + protected $migrator; + + /** @var \phpbb\config\config */ + protected $config; + + /** @var \phpbb\extension\manager */ + protected $extension_manager; + + public function getDataSet() + { + return $this->createXMLDataSet(__DIR__.'/fixtures/migration_remove_jabber.xml'); + } + + protected function setUp(): void + { + global $cache, $db, $phpbb_log, $phpbb_root_path, $phpEx, $skip_add_log, $table_prefix, $user; + + parent::setUp(); + + // Disable the logs + $skip_add_log = true; + + $db = $this->db = $this->new_dbal(); + $this->doctrine_db = $this->new_doctrine_dbal(); + $factory = new \phpbb\db\tools\factory(); + $this->db_tools = $factory->get($this->doctrine_db); + $this->db_tools->set_table_prefix($table_prefix); + $this->cache = new phpbb_mock_cache(); + $this->auth = new \phpbb\auth\auth(); + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $cache = $this->cache_service = new \phpbb\cache\service(new \phpbb\cache\driver\dummy(), new \phpbb\config\config(array()), $this->db, $phpbb_dispatcher, $phpbb_root_path, $phpEx); + + $this->config = new \phpbb\config\db($this->db, $this->cache, 'phpbb_config'); + $this->config->initialise($this->cache); + + $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); + $lang = new \phpbb\language\language($lang_loader); + $user = $this->user = new \phpbb\user($lang, '\phpbb\datetime'); + + $phpbb_log = new \phpbb\log\log($this->db, $this->user, $this->auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE); + + $container = new phpbb_mock_container_builder(); + $container->set('event_dispatcher', $phpbb_dispatcher); + + $finder_factory = $this->createMock('\phpbb\finder\factory'); + $this->extension_manager = new \phpbb\extension\manager( + $container, + $this->db, + $this->config, + $finder_factory, + 'phpbb_ext', + __DIR__ . '/../../phpBB/', + null + ); + + $module_manager = new \phpbb\module\module_manager($this->cache, $this->db, $this->extension_manager, 'phpbb_modules', $phpbb_root_path, $phpEx); + + $tools = array( + new \phpbb\db\migration\tool\config($this->config), + new \phpbb\db\migration\tool\module($this->db, $this->user, $module_manager, 'phpbb_modules'), + new \phpbb\db\migration\tool\permission($this->db, $this->cache_service, $this->auth, $phpbb_root_path, $phpEx), + ); + + $this->migrator = new \phpbb\db\migrator( + $container, + $this->config, + $this->db, + $this->db_tools, + 'phpbb_migrations', + __DIR__ . '/../../phpBB/', + 'php', + 'phpbb_', + self::get_core_tables(), + $tools, + new \phpbb\db\migration\helper() + ); + $container->set('migrator', $this->migrator); + + $remove_jabber_migration = $this->migrator->get_migration('\phpbb\db\migration\data\v400\remove_jabber'); + $depends = $remove_jabber_migration->depends_on(); + $this->migrator->populate_migrations($depends); + } + + public function test_remove_jabber_migration() + { + $sql = "SELECT id FROM phpbb_user_notifications + WHERE method = 'notification.method.jabber'"; + $result = $this->db->sql_query($sql); + $rowset = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); + $this->assertEquals(14, count($rowset)); + + $sql = "SELECT config_name FROM phpbb_config + WHERE config_name = 'jab_enable'"; + $this->assertNotFalse($this->db->sql_query($sql)); + + $sql = "SELECT auth_option FROM phpbb_acl_options + WHERE auth_option = 'a_jabber'"; + $this->assertNotFalse($this->db->sql_query($sql)); + + $this->migrator->set_migrations(['\phpbb\db\migration\data\v400\remove_jabber']); + + while (!$this->migrator->finished()) + { + try + { + $this->migrator->update(); + } + catch (\phpbb\db\migration\exception $e) + { + $this->fail('Applying migration error: ' . $e->__toString()); + } + } + + $sql = "SELECT id FROM phpbb_user_notifications + WHERE method = 'notification.method.jabber'"; + $this->db->sql_query($sql); + $this->assertFalse($this->db->sql_fetchfield('id')); + + $sql = "SELECT id FROM phpbb_user_notifications + WHERE method = 'notification.method.email'"; + $result = $this->db->sql_query($sql); + $rowset = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); + $this->assertEquals(14, count($rowset)); + + $sql = "SELECT config_name FROM phpbb_config + WHERE config_name = 'jab_enable'"; + $this->db->sql_query($sql); + $this->assertFalse($this->db->sql_fetchfield('config_name')); + + $sql = "SELECT auth_option FROM phpbb_acl_options + WHERE auth_option = 'a_jabber'"; + $this->db->sql_query($sql); + $this->assertFalse($this->db->sql_fetchfield('auth_option')); + + while ($this->migrator->migration_state('\phpbb\db\migration\data\v400\remove_jabber')) + { + try + { + $this->migrator->revert('\phpbb\db\migration\data\v400\remove_jabber'); + } + catch (\phpbb\db\migration\exception $e) + { + $this->fail('Reverting migration error: ' . $e->__toString()); + } + } + + $sql = "SELECT config_name FROM phpbb_config + WHERE config_name = 'jab_enable'"; + $this->db->sql_query($sql); + $this->assertEquals('jab_enable', $this->db->sql_fetchfield('config_name')); + + $sql = "SELECT auth_option FROM phpbb_acl_options + WHERE auth_option = 'a_jabber'"; + $this->db->sql_query($sql); + $this->assertEquals('a_jabber', $this->db->sql_fetchfield('auth_option')); + } +} From 0f71afae61de199c9108783a52892caccef988ea Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 6 Jul 2025 18:23:39 +0700 Subject: [PATCH 12/18] [ticket/17507] Add base class for testing migrations PHPBB-17507 --- tests/migrations/migration_test_base.php | 151 ++++++++++++++++++ .../remove_jabber_migration_test.php | 123 +------------- 2 files changed, 158 insertions(+), 116 deletions(-) create mode 100644 tests/migrations/migration_test_base.php diff --git a/tests/migrations/migration_test_base.php b/tests/migrations/migration_test_base.php new file mode 100644 index 0000000000..b7ccab3ae8 --- /dev/null +++ b/tests/migrations/migration_test_base.php @@ -0,0 +1,151 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +abstract class phpbb_migration_test_base extends phpbb_database_test_case +{ + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + + /** @var \Doctrine\DBAL\Connection */ + protected $doctrine_db; + + /** @var \phpbb\db\tools\tools_interface */ + protected $db_tools; + + /** @var \phpbb\db\migrator */ + protected $migrator; + + /** @var \phpbb\config\config */ + protected $config; + + /** @var \phpbb\extension\manager */ + protected $extension_manager; + + /** @var string */ + protected $fixture; + + /** @var string */ + protected $migration_class; + + public function getDataSet() + { + return $this->createXMLDataSet(__DIR__ . $this->fixture); + } + + protected function setUp(): void + { + global $cache, $db, $phpbb_log, $phpbb_root_path, $phpEx, $skip_add_log, $table_prefix, $user; + + parent::setUp(); + + // Disable the logs + $skip_add_log = true; + + $db = $this->db = $this->new_dbal(); + $this->doctrine_db = $this->new_doctrine_dbal(); + $factory = new \phpbb\db\tools\factory(); + $this->db_tools = $factory->get($this->doctrine_db); + $this->db_tools->set_table_prefix($table_prefix); + $this->cache = new phpbb_mock_cache(); + $this->auth = new \phpbb\auth\auth(); + $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); + $cache = $this->cache_service = new \phpbb\cache\service(new \phpbb\cache\driver\dummy(), new \phpbb\config\config(array()), $this->db, $phpbb_dispatcher, $phpbb_root_path, $phpEx); + + $this->config = new \phpbb\config\db($this->db, $this->cache, 'phpbb_config'); + $this->config->initialise($this->cache); + + $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); + $lang = new \phpbb\language\language($lang_loader); + $user = $this->user = new \phpbb\user($lang, '\phpbb\datetime'); + + $phpbb_log = new \phpbb\log\log($this->db, $this->user, $this->auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE); + + $container = new phpbb_mock_container_builder(); + $container->set('event_dispatcher', $phpbb_dispatcher); + + $finder_factory = $this->createMock('\phpbb\finder\factory'); + $this->extension_manager = new \phpbb\extension\manager( + $container, + $this->db, + $this->config, + $finder_factory, + 'phpbb_ext', + __DIR__ . '/../../phpBB/', + null + ); + + $module_manager = new \phpbb\module\module_manager($this->cache, $this->db, $this->extension_manager, 'phpbb_modules', $phpbb_root_path, $phpEx); + + $tools = array( + new \phpbb\db\migration\tool\config($this->config), + new \phpbb\db\migration\tool\config_text(new \phpbb\config\db_text($this->db, 'phpbb_config_text')), + new \phpbb\db\migration\tool\module($this->db, $this->user, $module_manager, 'phpbb_modules'), + new \phpbb\db\migration\tool\permission($this->db, $this->cache_service, $this->auth, $phpbb_root_path, $phpEx), + ); + + $this->migrator = new \phpbb\db\migrator( + $container, + $this->config, + $this->db, + $this->db_tools, + 'phpbb_migrations', + __DIR__ . '/../../phpBB/', + 'php', + 'phpbb_', + self::get_core_tables(), + $tools, + new \phpbb\db\migration\helper() + ); + $container->set('migrator', $this->migrator); + + $migration = $this->migrator->get_migration($this->migration_class); + $depends = $migration->depends_on(); + $this->migrator->populate_migrations($depends); + + $this->migrator->set_migrations([$this->migration_class]); + } + + protected function apply_migration() + { + while (!$this->migrator->finished()) + { + try + { + $this->migrator->update(); + } + catch (\phpbb\db\migration\exception $e) + { + $this->fail('Applying migration error: ' . $e->__toString()); + } + } + + return $this->migrator->finished(); + } + + protected function revert_migration() + { + while ($this->migrator->migration_state($this->migration_class)) + { + try + { + $this->migrator->revert($this->migration_class); + } + catch (\phpbb\db\migration\exception $e) + { + $this->fail('Reverting migration error: ' . $e->__toString()); + } + } + + return !$this->migrator->migration_state($this->migration_class); + } +} diff --git a/tests/migrations/remove_jabber_migration_test.php b/tests/migrations/remove_jabber_migration_test.php index c18b6ca754..2793ef032a 100644 --- a/tests/migrations/remove_jabber_migration_test.php +++ b/tests/migrations/remove_jabber_migration_test.php @@ -11,100 +11,13 @@ * */ -class phpbb_migrations_remove_jabber_migration_test extends phpbb_database_test_case +require_once __DIR__ . '/migration_test_base.php'; + +class phpbb_migrations_remove_jabber_migration_test extends phpbb_migration_test_base { - /** @var \phpbb\db\driver\driver_interface */ - protected $db; - /** @var \Doctrine\DBAL\Connection */ - protected $doctrine_db; - - /** @var \phpbb\db\tools\tools_interface */ - protected $db_tools; - - /** @var \phpbb\db\migrator */ - protected $migrator; - - /** @var \phpbb\config\config */ - protected $config; - - /** @var \phpbb\extension\manager */ - protected $extension_manager; - - public function getDataSet() - { - return $this->createXMLDataSet(__DIR__.'/fixtures/migration_remove_jabber.xml'); - } - - protected function setUp(): void - { - global $cache, $db, $phpbb_log, $phpbb_root_path, $phpEx, $skip_add_log, $table_prefix, $user; - - parent::setUp(); - - // Disable the logs - $skip_add_log = true; - - $db = $this->db = $this->new_dbal(); - $this->doctrine_db = $this->new_doctrine_dbal(); - $factory = new \phpbb\db\tools\factory(); - $this->db_tools = $factory->get($this->doctrine_db); - $this->db_tools->set_table_prefix($table_prefix); - $this->cache = new phpbb_mock_cache(); - $this->auth = new \phpbb\auth\auth(); - $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $cache = $this->cache_service = new \phpbb\cache\service(new \phpbb\cache\driver\dummy(), new \phpbb\config\config(array()), $this->db, $phpbb_dispatcher, $phpbb_root_path, $phpEx); - - $this->config = new \phpbb\config\db($this->db, $this->cache, 'phpbb_config'); - $this->config->initialise($this->cache); - - $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); - $lang = new \phpbb\language\language($lang_loader); - $user = $this->user = new \phpbb\user($lang, '\phpbb\datetime'); - - $phpbb_log = new \phpbb\log\log($this->db, $this->user, $this->auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE); - - $container = new phpbb_mock_container_builder(); - $container->set('event_dispatcher', $phpbb_dispatcher); - - $finder_factory = $this->createMock('\phpbb\finder\factory'); - $this->extension_manager = new \phpbb\extension\manager( - $container, - $this->db, - $this->config, - $finder_factory, - 'phpbb_ext', - __DIR__ . '/../../phpBB/', - null - ); - - $module_manager = new \phpbb\module\module_manager($this->cache, $this->db, $this->extension_manager, 'phpbb_modules', $phpbb_root_path, $phpEx); - - $tools = array( - new \phpbb\db\migration\tool\config($this->config), - new \phpbb\db\migration\tool\module($this->db, $this->user, $module_manager, 'phpbb_modules'), - new \phpbb\db\migration\tool\permission($this->db, $this->cache_service, $this->auth, $phpbb_root_path, $phpEx), - ); - - $this->migrator = new \phpbb\db\migrator( - $container, - $this->config, - $this->db, - $this->db_tools, - 'phpbb_migrations', - __DIR__ . '/../../phpBB/', - 'php', - 'phpbb_', - self::get_core_tables(), - $tools, - new \phpbb\db\migration\helper() - ); - $container->set('migrator', $this->migrator); - - $remove_jabber_migration = $this->migrator->get_migration('\phpbb\db\migration\data\v400\remove_jabber'); - $depends = $remove_jabber_migration->depends_on(); - $this->migrator->populate_migrations($depends); - } + protected $migration_class = '\phpbb\db\migration\data\v400\remove_jabber'; + protected $fixture = '/fixtures/migration_remove_jabber.xml'; public function test_remove_jabber_migration() { @@ -123,19 +36,7 @@ class phpbb_migrations_remove_jabber_migration_test extends phpbb_database_test_ WHERE auth_option = 'a_jabber'"; $this->assertNotFalse($this->db->sql_query($sql)); - $this->migrator->set_migrations(['\phpbb\db\migration\data\v400\remove_jabber']); - - while (!$this->migrator->finished()) - { - try - { - $this->migrator->update(); - } - catch (\phpbb\db\migration\exception $e) - { - $this->fail('Applying migration error: ' . $e->__toString()); - } - } + $this->apply_migration(); $sql = "SELECT id FROM phpbb_user_notifications WHERE method = 'notification.method.jabber'"; @@ -159,17 +60,7 @@ class phpbb_migrations_remove_jabber_migration_test extends phpbb_database_test_ $this->db->sql_query($sql); $this->assertFalse($this->db->sql_fetchfield('auth_option')); - while ($this->migrator->migration_state('\phpbb\db\migration\data\v400\remove_jabber')) - { - try - { - $this->migrator->revert('\phpbb\db\migration\data\v400\remove_jabber'); - } - catch (\phpbb\db\migration\exception $e) - { - $this->fail('Reverting migration error: ' . $e->__toString()); - } - } + $this->revert_migration(); $sql = "SELECT config_name FROM phpbb_config WHERE config_name = 'jab_enable'"; From 07b63fc6a8e2f0381536bf694a43e81af9746ed9 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 6 Jul 2025 19:01:06 +0700 Subject: [PATCH 13/18] [ticket/17507] Adjust module removal procedure PHPBB-17507 --- phpBB/phpbb/db/migration/tool/module.php | 2 +- .../fixtures/migration_remove_jabber.xml | 4 ++-- tests/migrations/migration_test_base.php | 16 +++++++-------- .../remove_jabber_migration_test.php | 20 +++++++++---------- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/phpBB/phpbb/db/migration/tool/module.php b/phpBB/phpbb/db/migration/tool/module.php index 2aeec109a4..8e35d49d03 100644 --- a/phpBB/phpbb/db/migration/tool/module.php +++ b/phpBB/phpbb/db/migration/tool/module.php @@ -377,7 +377,7 @@ class module implements \phpbb\db\migration\tool\tool_interface { if (!$this->exists($class, $parent, $module, true)) { - return; + throw new \phpbb\db\migration\exception('MODULE_NOT_EXIST', $module); } $parent_sql = ''; diff --git a/tests/migrations/fixtures/migration_remove_jabber.xml b/tests/migrations/fixtures/migration_remove_jabber.xml index 452c7fb40b..e263cdafcf 100644 --- a/tests/migrations/fixtures/migration_remove_jabber.xml +++ b/tests/migrations/fixtures/migration_remove_jabber.xml @@ -84,7 +84,7 @@ 1 acp - 0 + 1 48 59 ACP_CLIENT_COMMUNICATION @@ -120,7 +120,7 @@ 121 - u_sendim + u_sendim 1 0 0 diff --git a/tests/migrations/migration_test_base.php b/tests/migrations/migration_test_base.php index b7ccab3ae8..b2d9e3cf26 100644 --- a/tests/migrations/migration_test_base.php +++ b/tests/migrations/migration_test_base.php @@ -59,10 +59,10 @@ abstract class phpbb_migration_test_base extends phpbb_database_test_case $this->cache = new phpbb_mock_cache(); $this->auth = new \phpbb\auth\auth(); $phpbb_dispatcher = new phpbb_mock_event_dispatcher(); - $cache = $this->cache_service = new \phpbb\cache\service(new \phpbb\cache\driver\dummy(), new \phpbb\config\config(array()), $this->db, $phpbb_dispatcher, $phpbb_root_path, $phpEx); - $this->config = new \phpbb\config\db($this->db, $this->cache, 'phpbb_config'); $this->config->initialise($this->cache); + $cache = $this->cache_service = new \phpbb\cache\service($this->cache, $this->config, $this->db, $phpbb_dispatcher, $phpbb_root_path, $phpEx); + $lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx); $lang = new \phpbb\language\language($lang_loader); @@ -86,11 +86,11 @@ abstract class phpbb_migration_test_base extends phpbb_database_test_case $module_manager = new \phpbb\module\module_manager($this->cache, $this->db, $this->extension_manager, 'phpbb_modules', $phpbb_root_path, $phpEx); - $tools = array( - new \phpbb\db\migration\tool\config($this->config), - new \phpbb\db\migration\tool\config_text(new \phpbb\config\db_text($this->db, 'phpbb_config_text')), - new \phpbb\db\migration\tool\module($this->db, $this->user, $module_manager, 'phpbb_modules'), - new \phpbb\db\migration\tool\permission($this->db, $this->cache_service, $this->auth, $phpbb_root_path, $phpEx), + $this->tools = array( + 'config' => new \phpbb\db\migration\tool\config($this->config), + 'config_text' => new \phpbb\db\migration\tool\config_text(new \phpbb\config\db_text($this->db, 'phpbb_config_text')), + 'module' => new \phpbb\db\migration\tool\module($this->db, $this->user, $module_manager, 'phpbb_modules'), + 'permission' => new \phpbb\db\migration\tool\permission($this->db, $this->cache_service, $this->auth, $phpbb_root_path, $phpEx), ); $this->migrator = new \phpbb\db\migrator( @@ -103,7 +103,7 @@ abstract class phpbb_migration_test_base extends phpbb_database_test_case 'php', 'phpbb_', self::get_core_tables(), - $tools, + $this->tools, new \phpbb\db\migration\helper() ); $container->set('migrator', $this->migrator); diff --git a/tests/migrations/remove_jabber_migration_test.php b/tests/migrations/remove_jabber_migration_test.php index 2793ef032a..0a0c70b4ea 100644 --- a/tests/migrations/remove_jabber_migration_test.php +++ b/tests/migrations/remove_jabber_migration_test.php @@ -32,9 +32,9 @@ class phpbb_migrations_remove_jabber_migration_test extends phpbb_migration_test WHERE config_name = 'jab_enable'"; $this->assertNotFalse($this->db->sql_query($sql)); - $sql = "SELECT auth_option FROM phpbb_acl_options - WHERE auth_option = 'a_jabber'"; - $this->assertNotFalse($this->db->sql_query($sql)); + $this->assertTrue($this->tools['permission']->exists('a_jabber')); + $this->assertTrue($this->tools['permission']->exists('u_sendim')); + $this->assertTrue($this->tools['module']->exists('acp', 'ACP_CLIENT_COMMUNICATION', 'ACP_JABBER_SETTINGS')); $this->apply_migration(); @@ -55,10 +55,9 @@ class phpbb_migrations_remove_jabber_migration_test extends phpbb_migration_test $this->db->sql_query($sql); $this->assertFalse($this->db->sql_fetchfield('config_name')); - $sql = "SELECT auth_option FROM phpbb_acl_options - WHERE auth_option = 'a_jabber'"; - $this->db->sql_query($sql); - $this->assertFalse($this->db->sql_fetchfield('auth_option')); + $this->assertFalse($this->tools['permission']->exists('a_jabber')); + $this->assertFalse($this->tools['permission']->exists('u_sendim')); + $this->assertFalse($this->tools['module']->exists('acp', 'ACP_CLIENT_COMMUNICATION', 'ACP_JABBER_SETTINGS')); $this->revert_migration(); @@ -67,9 +66,8 @@ class phpbb_migrations_remove_jabber_migration_test extends phpbb_migration_test $this->db->sql_query($sql); $this->assertEquals('jab_enable', $this->db->sql_fetchfield('config_name')); - $sql = "SELECT auth_option FROM phpbb_acl_options - WHERE auth_option = 'a_jabber'"; - $this->db->sql_query($sql); - $this->assertEquals('a_jabber', $this->db->sql_fetchfield('auth_option')); + $this->assertTrue($this->tools['permission']->exists('a_jabber')); + $this->assertTrue($this->tools['permission']->exists('u_sendim')); + $this->assertTrue($this->tools['module']->exists('acp', 'ACP_CLIENT_COMMUNICATION', 'ACP_JABBER_SETTINGS')); } } From d2202b774773989dbd7bfbc0e50d32eda95ff59f Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 8 Jul 2025 00:48:57 +0700 Subject: [PATCH 14/18] [ticket/17507] Fix column removal issues PHPBB-17507 --- phpBB/phpbb/db/tools/doctrine.php | 138 ++++++++++-------- tests/migrations/migration_test_base.php | 2 +- .../remove_jabber_migration_test.php | 3 + 3 files changed, 84 insertions(+), 59 deletions(-) diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index 2d8908c9bc..8cead67885 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -300,34 +300,10 @@ class doctrine implements tools_interface */ public function sql_column_remove(string $table_name, string $column_name) { - // Check if this column is part of a primary key. If yes, remove the primary key. - $primary_key_indexes = $this->get_filtered_index_list($table_name, false); - - $primary_key_indexes = array_filter($primary_key_indexes, function($index) use ($column_name) { - $index_columns = array_map('strtolower', $index->getUnquotedColumns()); - return in_array($column_name, $index_columns, true) && $index->isPrimary(); - }); - - if (count($primary_key_indexes)) + $return = $this->schema_drop_primary_key($table_name, $column_name); + if ($return !== true) { - // For PostgreSQL, drop primary index first to avoid "Dependent objects still exist" error - if (stripos($this->get_schema_manager()->getDatabasePlatform()->getname(), 'postgresql') !== false) - { - $this->get_schema_manager()->dropIndex('"primary"', $table_name); - } - - $ret = $this->alter_schema( - function (Schema $schema) use ($table_name, $column_name): void - { - $table = $schema->getTable($table_name); - $table->dropPrimaryKey(); - } - ); - - if ($ret !== true) - { - return $ret; - } + return $return; } return $this->alter_schema( @@ -835,6 +811,8 @@ class doctrine implements tools_interface */ protected function schema_column_remove(Schema $schema, string $table_name, string $column_name, bool $safe_check = false): void { + $this->schema_drop_primary_key($table_name, $column_name); + $this->alter_table( $schema, $table_name, @@ -1005,6 +983,47 @@ class doctrine implements tools_interface $table->setPrimaryKey($columns); } + /** + * Removes primary key from a table + * + * @param string $table_name + * @param string $column_name + * + * @return bool|string[] + * + * @throws SchemaException + */ + protected function schema_drop_primary_key(string $table_name, string $column_name): array|bool + { + // Check if this column is part of a primary key. If yes, remove the primary key. + $primary_key_indexes = $this->get_filtered_index_list($table_name, false); + + $primary_key_indexes = array_filter($primary_key_indexes, function($index) use ($column_name) { + $index_columns = array_map('strtolower', $index->getUnquotedColumns()); + return in_array($column_name, $index_columns, true) && $index->isPrimary(); + }); + + $return = true; + if (count($primary_key_indexes)) + { + // For PostgreSQL, drop primary index first to avoid "Dependent objects still exist" error + if (stripos($this->get_schema_manager()->getDatabasePlatform()->getname(), 'postgresql') !== false) + { + $this->get_schema_manager()->dropIndex('"primary"', $table_name); + } + + $return = $this->alter_schema( + function (Schema $schema) use ($table_name, $column_name): void + { + $table = $schema->getTable($table_name); + $table->dropPrimaryKey(); + } + ); + } + + return $return; + } + /** * Recreate an index of a table * @@ -1016,40 +1035,43 @@ class doctrine implements tools_interface */ protected function recreate_index(Table $table, Index $index, array $new_columns): void { - if ($index->isPrimary()) + if (!empty($new_columns)) { - $table->dropPrimaryKey(); - } - else - { - $table->dropIndex($index->getName()); - } + if ($table->hasPrimaryKey() && $index->isPrimary()) + { + $table->dropPrimaryKey(); + } + else if ($table->hasIndex($index->getName())) + { + $table->dropIndex($index->getName()); + } - if (count($new_columns) > 0) - { - if ($index->isPrimary()) + if (count($new_columns) > 0) { - $table->setPrimaryKey( - $new_columns, - $index->getName(), - ); - } - else if ($index->isUnique()) - { - $table->addUniqueIndex( - $new_columns, - $index->getName(), - $index->getOptions(), - ); - } - else - { - $table->addIndex( - $new_columns, - $index->getName(), - $index->getFlags(), - $index->getOptions(), - ); + if ($index->isPrimary()) + { + $table->setPrimaryKey( + $new_columns, + $index->getName(), + ); + } + else if ($index->isUnique()) + { + $table->addUniqueIndex( + $new_columns, + $index->getName(), + $index->getOptions(), + ); + } + else + { + $table->addIndex( + $new_columns, + $index->getName(), + $index->getFlags(), + $index->getOptions(), + ); + } } } } diff --git a/tests/migrations/migration_test_base.php b/tests/migrations/migration_test_base.php index b2d9e3cf26..62148a6654 100644 --- a/tests/migrations/migration_test_base.php +++ b/tests/migrations/migration_test_base.php @@ -134,7 +134,7 @@ abstract class phpbb_migration_test_base extends phpbb_database_test_case protected function revert_migration() { - while ($this->migrator->migration_state($this->migration_class)) + while ($this->migrator->migration_state($this->migration_class) !== false) { try { diff --git a/tests/migrations/remove_jabber_migration_test.php b/tests/migrations/remove_jabber_migration_test.php index 0a0c70b4ea..e80a5cbbaa 100644 --- a/tests/migrations/remove_jabber_migration_test.php +++ b/tests/migrations/remove_jabber_migration_test.php @@ -69,5 +69,8 @@ class phpbb_migrations_remove_jabber_migration_test extends phpbb_migration_test $this->assertTrue($this->tools['permission']->exists('a_jabber')); $this->assertTrue($this->tools['permission']->exists('u_sendim')); $this->assertTrue($this->tools['module']->exists('acp', 'ACP_CLIENT_COMMUNICATION', 'ACP_JABBER_SETTINGS')); + + // Apply migration back + $this->apply_migration(); } } From c943ffaf0ec760bd10de0622387296ac41c4c40f Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 8 Jul 2025 19:23:59 +0700 Subject: [PATCH 15/18] [ticket/17507] Fix column removal issues PHPBB-17507 --- phpBB/phpbb/db/tools/doctrine.php | 117 ++++++++++-------------------- 1 file changed, 38 insertions(+), 79 deletions(-) diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index 8cead67885..45dbfd8ec8 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -300,12 +300,6 @@ class doctrine implements tools_interface */ public function sql_column_remove(string $table_name, string $column_name) { - $return = $this->schema_drop_primary_key($table_name, $column_name); - if ($return !== true) - { - return $return; - } - return $this->alter_schema( function (Schema $schema) use ($table_name, $column_name): void { @@ -811,7 +805,8 @@ class doctrine implements tools_interface */ protected function schema_column_remove(Schema $schema, string $table_name, string $column_name, bool $safe_check = false): void { - $this->schema_drop_primary_key($table_name, $column_name); + // $this->schema_drop_primary_key($table_name, $column_name); + // $this->schema_drop_unique_key($table_name, $column_name); $this->alter_table( $schema, @@ -983,47 +978,6 @@ class doctrine implements tools_interface $table->setPrimaryKey($columns); } - /** - * Removes primary key from a table - * - * @param string $table_name - * @param string $column_name - * - * @return bool|string[] - * - * @throws SchemaException - */ - protected function schema_drop_primary_key(string $table_name, string $column_name): array|bool - { - // Check if this column is part of a primary key. If yes, remove the primary key. - $primary_key_indexes = $this->get_filtered_index_list($table_name, false); - - $primary_key_indexes = array_filter($primary_key_indexes, function($index) use ($column_name) { - $index_columns = array_map('strtolower', $index->getUnquotedColumns()); - return in_array($column_name, $index_columns, true) && $index->isPrimary(); - }); - - $return = true; - if (count($primary_key_indexes)) - { - // For PostgreSQL, drop primary index first to avoid "Dependent objects still exist" error - if (stripos($this->get_schema_manager()->getDatabasePlatform()->getname(), 'postgresql') !== false) - { - $this->get_schema_manager()->dropIndex('"primary"', $table_name); - } - - $return = $this->alter_schema( - function (Schema $schema) use ($table_name, $column_name): void - { - $table = $schema->getTable($table_name); - $table->dropPrimaryKey(); - } - ); - } - - return $return; - } - /** * Recreate an index of a table * @@ -1035,43 +989,48 @@ class doctrine implements tools_interface */ protected function recreate_index(Table $table, Index $index, array $new_columns): void { - if (!empty($new_columns)) + if ($index->isPrimary()) { - if ($table->hasPrimaryKey() && $index->isPrimary()) + // For PostgreSQL, drop primary index first to avoid "Dependent objects still exist" error + if (stripos($this->connection->getDatabasePlatform()->getname(), 'postgresql') !== false) + { + $this->get_schema_manager()->dropUniqueConstraint($table->getPrimaryKey()->getName(), $table->getName()); + } + else { $table->dropPrimaryKey(); } - else if ($table->hasIndex($index->getName())) - { - $table->dropIndex($index->getName()); - } + } + else + { + $table->dropIndex($index->getName()); + } - if (count($new_columns) > 0) + if (count($new_columns) > 0) + { + if ($index->isPrimary()) { - if ($index->isPrimary()) - { - $table->setPrimaryKey( - $new_columns, - $index->getName(), - ); - } - else if ($index->isUnique()) - { - $table->addUniqueIndex( - $new_columns, - $index->getName(), - $index->getOptions(), - ); - } - else - { - $table->addIndex( - $new_columns, - $index->getName(), - $index->getFlags(), - $index->getOptions(), - ); - } + $table->setPrimaryKey( + $new_columns, + $index->getName(), + ); + } + else if ($index->isUnique()) + { + $table->addUniqueIndex( + $new_columns, + $index->getName(), + $index->getOptions(), + ); + } + else + { + $table->addIndex( + $new_columns, + $index->getName(), + $index->getFlags(), + $index->getOptions(), + ); } } } From 69a4990b2bc56cf5b6c7bd06463403361471e95b Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 9 Jul 2025 11:46:31 +0700 Subject: [PATCH 16/18] [ticket/17507] Fix PostgreSQL errors PHPBB-17507 --- .../phpbb/db/doctrine/postgresql_platform.php | 37 ++++++------------- phpBB/phpbb/db/tools/doctrine.php | 30 +++++---------- 2 files changed, 21 insertions(+), 46 deletions(-) diff --git a/phpBB/phpbb/db/doctrine/postgresql_platform.php b/phpBB/phpbb/db/doctrine/postgresql_platform.php index 2fa226b3ab..3108a040b3 100644 --- a/phpBB/phpbb/db/doctrine/postgresql_platform.php +++ b/phpBB/phpbb/db/doctrine/postgresql_platform.php @@ -14,8 +14,7 @@ namespace phpbb\db\doctrine; use Doctrine\DBAL\Platforms\AbstractPlatform; -use Doctrine\DBAL\Platforms\PostgreSQL94Platform; -use Doctrine\DBAL\Schema\Index; +use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\DBAL\Schema\Sequence; use Doctrine\DBAL\Schema\Table; use Doctrine\DBAL\Schema\TableDiff; @@ -32,7 +31,7 @@ use Doctrine\DBAL\Types\Type; * to stay compatible with the existing DB we have to change its * naming and not ours. */ -class postgresql_platform extends PostgreSQL94Platform +class postgresql_platform extends PostgreSQLPlatform { /** * {@inheritdoc} @@ -188,31 +187,19 @@ class postgresql_platform extends PostgreSQL94Platform { // If we have a primary or a unique index, we need to drop the constraint // instead of the index itself or postgreSQL will reject the query. - if ($index instanceof Index) + if (is_string($index) && $table !== null && $index === $this->tableName($table) . '_pkey') { - if ($index->isPrimary()) - { - if ($table instanceof Table) - { - $table = $table->getQuotedName($this); - } - else if (!is_string($table)) - { - throw new \InvalidArgumentException( - __METHOD__ . '() expects $table parameter to be string or ' . Table::class . '.' - ); - } - - return 'ALTER TABLE '.$table.' DROP CONSTRAINT '.$index->getQuotedName($this); - } - } - else if (! is_string($index)) - { - throw new \InvalidArgumentException( - __METHOD__ . '() expects $index parameter to be string or ' . Index::class . '.' - ); + return $this->getDropConstraintSQL($index, $this->tableName($table)); } return parent::getDropIndexSQL($index, $table); } + + /** + * {@inheritDoc} + */ + private function tableName($table) + { + return $table instanceof Table ? $table->getName() : (string) $table; + } } diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index 45dbfd8ec8..968b938ec0 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -96,7 +96,7 @@ class doctrine implements tools_interface */ protected function get_schema(): Schema { - return $this->get_schema_manager()->createSchema(); + return $this->get_schema_manager()->introspectSchema(); } /** @@ -380,7 +380,7 @@ class doctrine implements tools_interface { try { - $this->connection->executeQuery($this->get_schema_manager()->getDatabasePlatform()->getTruncateTableSQL($table_name)); + $this->connection->executeQuery($this->connection->getDatabasePlatform()->getTruncateTableSQL($table_name)); } catch (Exception $e) { @@ -485,7 +485,7 @@ class doctrine implements tools_interface $comparator = new comparator(); $schemaDiff = $comparator->compareSchemas($current_schema, $new_schema); - $queries = $schemaDiff->toSql($this->get_schema_manager()->getDatabasePlatform()); + $queries = $schemaDiff->toSql($this->connection->getDatabasePlatform()); if ($this->return_statements) { @@ -497,7 +497,6 @@ class doctrine implements tools_interface // executeQuery() must be used here because $query might return a result set, for instance REPAIR does $this->connection->executeQuery($query); } - return true; } @@ -629,7 +628,7 @@ class doctrine implements tools_interface $table = $schema->createTable($table_name); $short_table_name = table_helper::generate_shortname(self::remove_prefix($table_name, $this->table_prefix)); - $dbms_name = $this->get_schema_manager()->getDatabasePlatform()->getName(); + $dbms_name = $this->connection->getDatabasePlatform()->getName(); foreach ($table_data['COLUMNS'] as $column_name => $column_data) { @@ -646,7 +645,7 @@ class doctrine implements tools_interface ? [$table_data['PRIMARY_KEY']] : $table_data['PRIMARY_KEY']; - $table->setPrimaryKey($table_data['PRIMARY_KEY']); + $table->setPrimaryKey($table_data['PRIMARY_KEY'], false); } if (array_key_exists('KEYS', $table_data)) @@ -728,7 +727,7 @@ class doctrine implements tools_interface return false; } - $dbms_name = $this->get_schema_manager()->getDatabasePlatform()->getName(); + $dbms_name = $this->connection->getDatabasePlatform()->getName(); list($type, $options) = table_helper::convert_column_data($column_data, $dbms_name); $table->addColumn($column_name, $type, $options); @@ -760,7 +759,7 @@ class doctrine implements tools_interface return; } - $dbms_name = $this->get_schema_manager()->getDatabasePlatform()->getName(); + $dbms_name = $this->connection->getDatabasePlatform()->getName(); list($type, $options) = table_helper::convert_column_data($column_data, $dbms_name); $options['type'] = Type::getType($type); @@ -805,13 +804,10 @@ class doctrine implements tools_interface */ protected function schema_column_remove(Schema $schema, string $table_name, string $column_name, bool $safe_check = false): void { - // $this->schema_drop_primary_key($table_name, $column_name); - // $this->schema_drop_unique_key($table_name, $column_name); - $this->alter_table( $schema, $table_name, - function (Table $table) use ($schema, $table_name, $column_name, $safe_check): void + function (Table $table) use (&$schema, $table_name, $column_name, $safe_check): void { if ($safe_check && !$table->hasColumn($column_name)) { @@ -991,15 +987,7 @@ class doctrine implements tools_interface { if ($index->isPrimary()) { - // For PostgreSQL, drop primary index first to avoid "Dependent objects still exist" error - if (stripos($this->connection->getDatabasePlatform()->getname(), 'postgresql') !== false) - { - $this->get_schema_manager()->dropUniqueConstraint($table->getPrimaryKey()->getName(), $table->getName()); - } - else - { - $table->dropPrimaryKey(); - } + $table->dropPrimaryKey(); } else { From b9dbce21f780709b709d6dfb9f8ed5f6e518222a Mon Sep 17 00:00:00 2001 From: rxu Date: Wed, 9 Jul 2025 11:59:53 +0700 Subject: [PATCH 17/18] [ticket/17507] Minor code fixes PHPBB-17507 --- phpBB/phpbb/db/tools/doctrine.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index 968b938ec0..87628d440e 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -96,7 +96,7 @@ class doctrine implements tools_interface */ protected function get_schema(): Schema { - return $this->get_schema_manager()->introspectSchema(); + return $this->get_schema_manager()->introspectSchema(); } /** @@ -645,7 +645,7 @@ class doctrine implements tools_interface ? [$table_data['PRIMARY_KEY']] : $table_data['PRIMARY_KEY']; - $table->setPrimaryKey($table_data['PRIMARY_KEY'], false); + $table->setPrimaryKey($table_data['PRIMARY_KEY']); } if (array_key_exists('KEYS', $table_data)) @@ -807,7 +807,7 @@ class doctrine implements tools_interface $this->alter_table( $schema, $table_name, - function (Table $table) use (&$schema, $table_name, $column_name, $safe_check): void + function (Table $table) use ($schema, $table_name, $column_name, $safe_check): void { if ($safe_check && !$table->hasColumn($column_name)) { From 40cd938e51f7fec9f76a1545f81c6dc2a9ca1723 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 13 Jul 2025 14:52:42 +0700 Subject: [PATCH 18/18] [ticket/17528] Review code fix PHPBB-17528 --- phpBB/phpbb/db/doctrine/postgresql_platform.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/db/doctrine/postgresql_platform.php b/phpBB/phpbb/db/doctrine/postgresql_platform.php index 3108a040b3..d67b2439b2 100644 --- a/phpBB/phpbb/db/doctrine/postgresql_platform.php +++ b/phpBB/phpbb/db/doctrine/postgresql_platform.php @@ -95,7 +95,7 @@ class postgresql_platform extends PostgreSQLPlatform { $sequence = new Sequence($this->getIdentitySequenceName($table_name, $column_name)); $sequence_sql[] = $this->getCreateSequenceSQL($sequence); - $post_sql[] = 'ALTER SEQUENCE '.$sequence->getName().' OWNED BY ' . $table_name . '.' . $column_name; + $post_sql[] = 'ALTER SEQUENCE ' . $sequence->getName() . ' OWNED BY ' . $table_name . '.' . $column_name; } } $sql = array_merge($sequence_sql, $sql, $post_sql);