From ba0282bf38fff65205c23e60a2d2055b9b58913d Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 10 Jun 2025 10:16:02 +0700 Subject: [PATCH 1/4] [ticket/17524] Add possibility to use index key length in migrations PHPBB-17524 --- phpBB/phpbb/db/tools/doctrine.php | 51 ++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index bd3f71f83a..73d47117ce 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -629,25 +629,16 @@ class doctrine implements tools_interface foreach ($table_data['KEYS'] as $key_name => $key_data) { $columns = (is_array($key_data[1])) ? $key_data[1] : [$key_data[1]]; - - // Supports key columns defined with there length - $columns = array_map(function (string $column) - { - if (strpos($column, ':') !== false) - { - $parts = explode(':', $column, 2); - return $parts[0]; - } - return $column; - }, $columns); + $options = []; + $this->schema_get_index_key_data($columns, $options); if ($key_data[0] === 'UNIQUE') { - $table->addUniqueIndex($columns, $key_name); + $table->addUniqueIndex($columns, $key_name, $options); } else { - $table->addIndex($columns, $key_name); + $table->addIndex($columns, $key_name, [], $options); } } } @@ -830,7 +821,10 @@ class doctrine implements tools_interface return; } - $table->addIndex($columns, $index_name); + $options = []; + $this->schema_get_index_key_data($columns, $options); + + $table->addIndex($columns, $index_name, [], $options); } /** @@ -852,7 +846,10 @@ class doctrine implements tools_interface return; } - $table->addUniqueIndex($columns, $index_name); + $options = []; + $this->schema_get_index_key_data($columns, $options); + + $table->addUniqueIndex($columns, $index_name, $options); } /** @@ -891,6 +888,30 @@ class doctrine implements tools_interface $table->setPrimaryKey($columns); } + /** + * Checks if index data contains key length + * and put it into $options['lengths'] array. + * Handles key length in formats of 'keyname:123' or 'keyname(123)' + * + * @param array $columns + * @param array $options + */ + protected function schema_get_index_key_data(array &$columns, array &$options): void + { + if (!empty($columns)) + { + $columns = array_map(function (string $column) use (&$options) + { + if (preg_match('/^([a-zA-Z0-9_]+)(?:(?:\:|\()([0-9]{1,3})\)?)?$/', $column, $parts)) + { + $options['lengths'][] = $parts[2] ?? null; + return $parts[1]; + } + return $column; + }, $columns); + } + } + /** * Recreate an index of a table * From af93b1aee997502bae38b2590ecfac5a31d45006 Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 10 Jun 2025 11:31:48 +0700 Subject: [PATCH 2/4] [ticket/17524] Add index data getter to db tools PHPBB-17524 --- phpBB/phpbb/db/tools/doctrine.php | 44 +++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index 73d47117ce..c7616a2a27 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -378,6 +378,46 @@ class doctrine implements tools_interface } } + /** + * Returns an array of the table index names and relevant data in format + * [ + * [$index_name] = [ + * 'columns' => (array) $index_columns, + * 'flags' => (array) $index_flags, + * 'options' => (array) $index_options, + * 'is_primary'=> (bool) $isPrimary, + * 'is_unique' => (bool) $isUnique, + * 'is_simple' => (bool) $isSimple, + * ] + * + * @param Schema $schema + * @param string $table_name + * @param string $column_name + * @param array $column_data + * @param bool $safe_check + * + * @throws SchemaException + */ + public function sql_get_table_index_data(string $table_name): array + { + $schema = $this->get_schema(); + $table = $schema->getTable($table_name); + $indexes = []; + foreach ($table->getIndexes() as $index) + { + $indexes[$index->getName()] = [ + 'columns' => array_map('strtolower', $index->getUnquotedColumns()), + 'flags' => $index->getFlags(), + 'options' => $index->getOptions(), + 'is_primary'=> $index->isPrimary(), + 'is_unique' => $index->isUnique(), + 'is_simple' => $index->isSimpleIndex(), + ]; + } + + return $indexes; + } + /** * Returns an array of indices for either unique and primary keys, or simple indices. * @@ -908,8 +948,8 @@ class doctrine implements tools_interface return $parts[1]; } return $column; - }, $columns); - } + }, $columns); + } } /** From 83c6ffce6427d7101632c85c53865d289c5f4ec4 Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 10 Jun 2025 13:55:48 +0700 Subject: [PATCH 3/4] [ticket/17524] Add index migrator tests PHPBB-17524 --- phpBB/phpbb/db/tools/doctrine.php | 6 +--- tests/dbal/migration/schema.php | 20 ++++++++++---- tests/dbal/migrator_test.php | 46 +++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 10 deletions(-) diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index c7616a2a27..9cc8ed3f6f 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -390,13 +390,9 @@ class doctrine implements tools_interface * 'is_simple' => (bool) $isSimple, * ] * - * @param Schema $schema * @param string $table_name - * @param string $column_name - * @param array $column_data - * @param bool $safe_check * - * @throws SchemaException + * @return array */ public function sql_get_table_index_data(string $table_name): array { diff --git a/tests/dbal/migration/schema.php b/tests/dbal/migration/schema.php index 38663bcc16..d91ed6a316 100644 --- a/tests/dbal/migration/schema.php +++ b/tests/dbal/migration/schema.php @@ -22,12 +22,22 @@ class phpbb_dbal_migration_schema extends \phpbb\db\migration\migration ), ), 'add_tables' => array( - $this->table_prefix . 'foobar' => array( - 'COLUMNS' => array( - 'module_id' => array('UINT:3', NULL, 'auto_increment'), - ), + $this->table_prefix . 'foobar' => [ + 'COLUMNS' => [ + 'module_id' => ['UINT:3', NULL, 'auto_increment'], + 'user_id' => ['ULINT', 0], + 'endpoint' => ['TEXT', ''], + 'expiration_time' => ['TIMESTAMP', 0], + 'p256dh' => ['VCHAR', ''], + 'auth' => ['VCHAR:100', ''], + ], 'PRIMARY_KEY' => 'module_id', - ), + 'KEYS' => [ + 'i_simple' => ['INDEX', ['user_id', 'endpoint:191']], + 'i_uniq' => ['UNIQUE', ['expiration_time', 'p256dh(100)']], + 'i_auth' => ['INDEX', 'auth'], + ], + ], ), ); } diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php index 5d16fa63ee..037002c4d0 100644 --- a/tests/dbal/migrator_test.php +++ b/tests/dbal/migrator_test.php @@ -408,6 +408,52 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case $this->assertTrue($this->db_tools->sql_column_exists('phpbb_config', 'test_column1')); $this->assertTrue($this->db_tools->sql_table_exists('phpbb_foobar')); + $index_data_row = $this->db_tools->sql_get_table_index_data('phpbb_foobar'); + foreach ($index_data_row as $index_name => $index_data) + { + switch ($index_name) + { + case 'i_simple': + $this->assertEquals(['user_id', 'endpoint'], $index_data['columns']); + $this->assertEmpty($index_data['flags']); + $this->assertFalse($index_data['is_primary']); + $this->assertFalse($index_data['is_unique']); + $this->assertTrue($index_data['is_simple']); + $this->assertEquals(2, count($index_data['options']['lengths'])); + $this->assertEmpty($index_data['options']['lengths'][0]); + $this->assertEquals(191, $index_data['options']['lengths'][1]); + break; + case 'i_uniq': + $this->assertEquals(['expiration_time', 'p256dh'], $index_data['columns']); + $this->assertEmpty($index_data['flags']); + $this->assertFalse($index_data['is_primary']); + $this->assertTrue($index_data['is_unique']); + $this->assertFalse($index_data['is_simple']); + $this->assertEquals(2, count($index_data['options']['lengths'])); + $this->assertEmpty($index_data['options']['lengths'][0]); + $this->assertEquals(100, $index_data['options']['lengths'][1]); + break; + case 'i_auth': + $this->assertEquals(['auth'], $index_data['columns']); + $this->assertEmpty($index_data['flags']); + $this->assertFalse($index_data['is_primary']); + $this->assertFalse($index_data['is_unique']); + $this->assertTrue($index_data['is_simple']); + $this->assertEquals(1, count($index_data['options']['lengths'])); + $this->assertEmpty($index_data['options']['lengths'][0]); + break; + default: // Primary key + $this->assertEquals(['module_id'], $index_data['columns']); + $this->assertEmpty($index_data['flags']); + $this->assertTrue($index_data['is_primary']); + $this->assertTrue($index_data['is_unique']); + $this->assertFalse($index_data['is_simple']); + $this->assertEquals(1, count($index_data['options']['lengths'])); + $this->assertEmpty($index_data['options']['lengths'][0]); + break; + } + } + while ($this->migrator->migration_state('phpbb_dbal_migration_schema')) { $this->migrator->revert('phpbb_dbal_migration_schema'); From 993e637af9426c2afd3ac6a2662512068eb7d9d3 Mon Sep 17 00:00:00 2001 From: rxu Date: Tue, 10 Jun 2025 15:22:46 +0700 Subject: [PATCH 4/4] [ticket/17524] test index key length only on MySQL where it does apply PHPBB-17524 --- tests/dbal/migrator_test.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php index 037002c4d0..6bd3854a67 100644 --- a/tests/dbal/migrator_test.php +++ b/tests/dbal/migrator_test.php @@ -409,6 +409,7 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case $this->assertTrue($this->db_tools->sql_table_exists('phpbb_foobar')); $index_data_row = $this->db_tools->sql_get_table_index_data('phpbb_foobar'); + $mysql = $this->db->get_sql_layer() === 'mysqli'; // Index length only applies to MySQL indexes foreach ($index_data_row as $index_name => $index_data) { switch ($index_name) @@ -421,7 +422,7 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case $this->assertTrue($index_data['is_simple']); $this->assertEquals(2, count($index_data['options']['lengths'])); $this->assertEmpty($index_data['options']['lengths'][0]); - $this->assertEquals(191, $index_data['options']['lengths'][1]); + $this->assertEquals($mysql ? 191 : null, $index_data['options']['lengths'][1]); break; case 'i_uniq': $this->assertEquals(['expiration_time', 'p256dh'], $index_data['columns']); @@ -431,7 +432,7 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case $this->assertFalse($index_data['is_simple']); $this->assertEquals(2, count($index_data['options']['lengths'])); $this->assertEmpty($index_data['options']['lengths'][0]); - $this->assertEquals(100, $index_data['options']['lengths'][1]); + $this->assertEquals($mysql ? 100 : null, $index_data['options']['lengths'][1]); break; case 'i_auth': $this->assertEquals(['auth'], $index_data['columns']);