diff --git a/phpBB/includes/acp/acp_extensions.php b/phpBB/includes/acp/acp_extensions.php index e08c2e341b..626f1b09c1 100644 --- a/phpBB/includes/acp/acp_extensions.php +++ b/phpBB/includes/acp/acp_extensions.php @@ -276,6 +276,12 @@ class acp_extensions { $this->template->assign_var('MIGRATOR_ERROR', $e->getLocalisedMessage($this->user)); } + catch (\Exception $e) + { + $stack_trace = phpbb_filter_root_path(str_replace("\n", '
', $e->getTraceAsString())); + $message = $e->getMessage(); + $this->template->assign_var('MIGRATOR_ERROR', '' . $message . '

' . $stack_trace); + } $this->tpl_name = 'acp_ext_enable'; diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index f725d5f996..72241b52f0 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -2716,7 +2716,7 @@ function get_backtrace() // Only show function arguments for include etc. // Other parameters may contain sensible information $argument = ''; - if (!empty($trace['args'][0]) && in_array($trace['function'], array('include', 'require', 'include_once', 'require_once'))) + if (!empty($trace['args'][0]) && in_array($trace['function'], array('include', 'require', 'include_once', 'require_once', 'try_apply'))) { $argument = htmlspecialchars(phpbb_filter_root_path($trace['args'][0]), ENT_COMPAT); } diff --git a/phpBB/install/startup.php b/phpBB/install/startup.php index b8ceb37342..0cff634795 100644 --- a/phpBB/install/startup.php +++ b/phpBB/install/startup.php @@ -107,7 +107,7 @@ function installer_msg_handler($errno, $msg_text, $errfile, $errline): bool { /** @var \phpbb\install\helper\iohandler\iohandler_interface $iohandler */ $iohandler = $phpbb_installer_container->get('installer.helper.iohandler'); - $iohandler->add_error_message($msg); + $iohandler->add_error_message($msg, get_backtrace()); $iohandler->send_response(true); exit(); } diff --git a/phpBB/phpbb/db/doctrine/table_helper.php b/phpBB/phpbb/db/doctrine/table_helper.php index 935db35838..ae80e23a50 100644 --- a/phpBB/phpbb/db/doctrine/table_helper.php +++ b/phpBB/phpbb/db/doctrine/table_helper.php @@ -123,6 +123,54 @@ class table_helper } } + /** + * Maps table names to thair short names for the purpose of prefixing tables' index names. + * + * @param array $table_names Table names with prefix to add to the map. + * @param string $table_prefix Tables prefix. + * + * @return array Pairs of table names and their short name representations. + * @psalm-return array{string, string} + */ + public static function map_short_table_names(array $table_names = [], string $table_prefix = ''): array + { + $short_table_names_map = []; + foreach ($table_names as $table_name) + { + $short_table_names_map[$table_name] = self::generate_shortname(str_replace($table_prefix, '', $table_name)); + } + + return $short_table_names_map; + } + + /** + * Generates short table names for the purpose of prefixing tables' index names. + * + * @param string $table_name Table name without prefix to generate its short name. + * + * @return string Short table name. + */ + public static function generate_shortname(string $table_name = ''): string + { + // Only shorten actually long names + if (strlen($table_name) > 4) + { + // Remove vowels + $table_name = preg_replace('/([^aeiou_])([aeiou]+)/i', '$1', $table_name); + + // Remove underscores + $table_name = str_replace('_', '', $table_name); + + // Remove repeated consonants and their combinations (like 'ss', 'flfl' and similar) + $table_name = preg_replace('/(.+)\\1+/i', '$1', $table_name); + + // Restrict short name length to 10 chars + $table_name = substr($table_name, 0, 10); + } + + return $table_name; + } + /** * Private constructor. Call methods of table_helper statically. */ diff --git a/phpBB/phpbb/db/migration/data/v400/rename_duplicated_index_names.php b/phpBB/phpbb/db/migration/data/v400/rename_duplicated_index_names.php new file mode 100644 index 0000000000..dc053a0f77 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v400/rename_duplicated_index_names.php @@ -0,0 +1,129 @@ + + * @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\migration\data\v400; + +use phpbb\db\migration\migration; +use phpbb\db\doctrine\table_helper; + +class rename_duplicated_index_names extends migration +{ + public static function depends_on() + { + return [ + '\phpbb\db\migration\data\v400\storage_track_index', + ]; + } + + public function update_schema() + { + $rename_index = []; + $is_prefixed_index = false; + $tables_index_names = $this->get_tables_index_names(); + $short_table_names = table_helper::map_short_table_names(array_keys($tables_index_names), $this->table_prefix); + + foreach ($tables_index_names as $table_name => $key_names) + { + foreach ($key_names as $key_name) + { + $prefixless_table_name = strpos($table_name, $this->table_prefix) === 0 ? substr($table_name, strlen($this->table_prefix)) : $table_name; + + // Check if there's at least one index name is prefixed, otherwise we operate on generated database schema + $is_prefixed_index = $is_prefixed_index || (strpos($key_name, $table_name) === 0); + + // If key name is prefixed by its table name (with or without tables prefix), remove that key name prefix. + $cleaned_key_name = !$is_prefixed_index ? $key_name : str_replace(strpos($key_name, $table_name) === 0 ? $table_name . '_' : $prefixless_table_name . '_', '', $key_name); + + $key_name_new = $short_table_names[$table_name] . '_' . $cleaned_key_name; + $rename_index[$table_name][$key_name !== $cleaned_key_name ? $key_name : $cleaned_key_name] = $key_name_new; + } + } + + return [ + 'rename_index' => $rename_index, + ]; + } + + public function revert_schema() + { + $schema = $this->update_schema(); + array_walk($schema['rename_index'], function (&$index_data, $table_name) { + $index_data = array_flip($index_data); + }); + + return $schema; + } + + protected function get_schema() + { + $self_classname = '\\' . str_replace('/', '\\', self::class); + $finder_factory = new \phpbb\finder\factory(null, false, $this->phpbb_root_path, $this->php_ext); + $finder = $finder_factory->get(); + $migrator_classes = $finder->core_path('phpbb/db/migration/data/')->get_classes(); + $self_class_index = array_search($self_classname, $migrator_classes); + unset($migrator_classes[$self_class_index]); + + $schema_generator = new \phpbb\db\migration\schema_generator( + $migrator_classes, + $this->config, + $this->db, + $this->db_tools, + $this->phpbb_root_path, + $this->php_ext, + $this->table_prefix, + $this->tables + ); + + return $schema_generator->get_schema(); + } + + public function get_tables_index_names() + { + $table_keys = []; + $schema_manager = $this->db_tools->get_connection()->createSchemaManager(); + $table_names = $schema_manager->listTableNames(); + + if (!empty($table_names)) + { + foreach ($table_names as $table_name) + { + $indices = $schema_manager->listTableIndexes($table_name); + + $index_names = array_keys( + array_filter($indices, function (\Doctrine\DBAL\Schema\Index $index) + { + return !$index->isPrimary(); + }) + ); + + if (!empty($index_names)) + { + $table_keys[$table_name] = $index_names; + } + } + } + else + { + $db_table_schema = $this->get_schema(); + foreach ($db_table_schema as $table_name => $table_data) + { + if (isset($table_data['KEYS'])) + { + $table_keys[$table_name] = array_keys($table_data['KEYS']); + } + } + } + + return $table_keys; + } +} diff --git a/phpBB/phpbb/db/migration/helper.php b/phpBB/phpbb/db/migration/helper.php index bce2efff51..44cd23462a 100644 --- a/phpBB/phpbb/db/migration/helper.php +++ b/phpBB/phpbb/db/migration/helper.php @@ -42,6 +42,7 @@ class helper 'add_primary_keys' => 2, // perform_schema_changes only uses one level, but second is in the function 'add_unique_index' => 2, 'add_index' => 2, + 'rename_index' => 1, ); foreach ($nested_level as $change_type => $data_depth) diff --git a/phpBB/phpbb/db/migration/schema_generator.php b/phpBB/phpbb/db/migration/schema_generator.php index ce07c30d45..90cfddbe5a 100644 --- a/phpBB/phpbb/db/migration/schema_generator.php +++ b/phpBB/phpbb/db/migration/schema_generator.php @@ -187,6 +187,7 @@ class schema_generator 'add_index' => 'KEYS', 'add_unique_index' => 'KEYS', 'drop_keys' => 'KEYS', + 'rename_index' => 'KEYS', ]; $schema_changes = $migration->update_schema(); @@ -206,6 +207,7 @@ class schema_generator { case 'add': case 'change': + case 'rename': $action = function(&$value, $changes, $value_transform = null) { self::set_all($value, $changes, $value_transform); }; @@ -347,7 +349,7 @@ class schema_generator */ private static function get_value_transform(string $change_type, string $schema_type) : Closure|null { - if ($change_type !== 'add') + if (!in_array($change_type, ['add', 'rename'])) { return null; } @@ -355,6 +357,23 @@ class schema_generator switch ($schema_type) { case 'index': + if ($change_type == 'rename') + { + return function(&$value, $key, $change) { + if (isset($value[$key])) + { + $data_backup = $value[$key]; + unset($value[$key]); + $value[$change] = $data_backup; + unset($data_backup); + } + else + { + return null; + } + }; + } + return function(&$value, $key, $change) { $value[$key] = ['INDEX', $change]; }; diff --git a/phpBB/phpbb/db/tools/doctrine.php b/phpBB/phpbb/db/tools/doctrine.php index aaf44aed68..3853eae9cb 100644 --- a/phpBB/phpbb/db/tools/doctrine.php +++ b/phpBB/phpbb/db/tools/doctrine.php @@ -61,6 +61,14 @@ class doctrine implements tools_interface $this->connection = $connection; } + /** + * {@inheritDoc} + */ + public function get_connection(): Connection + { + return $this->connection; + } + /** * @return AbstractSchemaManager * @@ -160,6 +168,7 @@ class doctrine implements tools_interface */ public function sql_index_exists(string $table_name, string $index_name): bool { + $index_name = self::normalize_index_name($table_name, $index_name); return $this->asset_exists($index_name, $this->get_filtered_index_list($table_name, true)); } @@ -168,6 +177,7 @@ class doctrine implements tools_interface */ public function sql_unique_index_exists(string $table_name, string $index_name): bool { + $index_name = self::normalize_index_name($table_name, $index_name); return $this->asset_exists($index_name, $this->get_filtered_index_list($table_name, false)); } @@ -322,6 +332,7 @@ class doctrine implements tools_interface */ public function sql_create_index(string $table_name, string $index_name, $column) { + $index_name = self::normalize_index_name($table_name, $index_name); return $this->alter_schema( function (Schema $schema) use ($table_name, $index_name, $column): void { @@ -330,11 +341,27 @@ class doctrine implements tools_interface ); } + /** + * {@inheritDoc} + */ + public function sql_rename_index(string $table_name, string $index_name_old, string $index_name_new) + { + $index_name_old = self::normalize_index_name($table_name, $index_name_old); + $index_name_new = self::normalize_index_name($table_name, $index_name_new); + return $this->alter_schema( + function (Schema $schema) use ($table_name, $index_name_old, $index_name_new): void + { + $this->schema_rename_index($schema, $table_name, $index_name_old, $index_name_new); + } + ); + } + /** * {@inheritDoc} */ public function sql_index_drop(string $table_name, string $index_name) { + $index_name = self::normalize_index_name($table_name, $index_name); return $this->alter_schema( function (Schema $schema) use ($table_name, $index_name): void { @@ -348,6 +375,7 @@ class doctrine implements tools_interface */ public function sql_create_unique_index(string $table_name, string $index_name, $column) { + $index_name = self::normalize_index_name($table_name, $index_name); return $this->alter_schema( function (Schema $schema) use ($table_name, $index_name, $column): void { @@ -384,6 +412,47 @@ class doctrine implements tools_interface } } + /** + * {@inheritDoc} + */ + public static function normalize_index_name(string $table_name, string $index_name, bool $remove_prefix = false): string + { + $prefixless_table_name = self::remove_prefix($table_name); + $short_table_name = table_helper::generate_shortname($prefixless_table_name); + if (!self::is_prefixed($index_name, $short_table_name . '_') + && !self::is_prefixed($index_name, $prefixless_table_name . '_') + && !self::is_prefixed($index_name, $table_name . '_') + ) + { + $index_name = $short_table_name . '_' . $index_name; + } + else if ($remove_prefix) + { + $index_name = self::remove_prefix($index_name); + } + + return $index_name; + } + + /** + * {@inheritDoc} + */ + public static function remove_prefix(string $name): string + { + $prefix_end_pos = strpos($name, '_'); + $prefixless_name = substr($name, $prefix_end_pos + 1); + + return $prefixless_name; + } + + /** + * {@inheritDoc} + */ + public static function is_prefixed(string $name, string $prefix): bool + { + return strpos($name, $prefix) === 0; + } + /** * Returns an array of indices for either unique and primary keys, or simple indices. * @@ -458,34 +527,26 @@ class doctrine implements tools_interface */ protected function alter_schema(callable $callback) { - try + $current_schema = $this->get_schema(); + $new_schema = clone $current_schema; + call_user_func($callback, $new_schema); + + $comparator = new comparator(); + $schemaDiff = $comparator->compareSchemas($current_schema, $new_schema); + $queries = $schemaDiff->toSql($this->get_schema_manager()->getDatabasePlatform()); + + if ($this->return_statements) { - $current_schema = $this->get_schema(); - $new_schema = clone $current_schema; - call_user_func($callback, $new_schema); - - $comparator = new comparator(); - $schemaDiff = $comparator->compareSchemas($current_schema, $new_schema); - $queries = $schemaDiff->toSql($this->get_schema_manager()->getDatabasePlatform()); - - if ($this->return_statements) - { - return $queries; - } - - foreach ($queries as $query) - { - // executeQuery() must be used here because $query might return a result set, for instance REPAIR does - $this->connection->executeQuery($query); - } - - return true; + return $queries; } - catch (Exception $e) + + foreach ($queries as $query) { - // @todo: check if it makes sense to properly handle the exception - return [$e->getMessage()]; + // executeQuery() must be used here because $query might return a result set, for instance REPAIR does + $this->connection->executeQuery($query); } + + return true; } /** @@ -553,6 +614,11 @@ class doctrine implements tools_interface 'use_key' => true, 'per_table' => true, ], + 'rename_index' => [ + 'method' => 'schema_rename_index', + 'use_key' => true, + 'per_table' => true, + ], ]; foreach ($mapping as $action => $params) @@ -635,6 +701,7 @@ 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]]; + $key_name = self::normalize_index_name($table_name, $key_name); // Supports key columns defined with there length $columns = array_map(function (string $column) @@ -830,6 +897,7 @@ class doctrine implements tools_interface { $columns = (is_array($column)) ? $column : [$column]; $table = $schema->getTable($table_name); + $index_name = self::normalize_index_name($table_name, $index_name); if ($safe_check && $table->hasIndex($index_name)) { @@ -839,6 +907,28 @@ class doctrine implements tools_interface $table->addIndex($columns, $index_name); } + /** + * @param Schema $schema + * @param string $table_name + * @param string $index_name_old + * @param string $index_name_new + * @param bool $safe_check + * + * @throws SchemaException + */ + protected function schema_rename_index(Schema $schema, string $table_name, string $index_name_old, string $index_name_new, bool $safe_check = false): void + { + $table = $schema->getTable($table_name); + $index_name_new = self::normalize_index_name($table_name, $index_name_new); + + if ($safe_check && !$table->hasIndex($index_name_old)) + { + return; + } + + $table->renameIndex($index_name_old, $index_name_new); + } + /** * @param Schema $schema * @param string $table_name @@ -852,6 +942,7 @@ class doctrine implements tools_interface { $columns = (is_array($column)) ? $column : [$column]; $table = $schema->getTable($table_name); + $index_name = self::normalize_index_name($table_name, $index_name); if ($safe_check && $table->hasIndex($index_name)) { @@ -872,6 +963,7 @@ class doctrine implements tools_interface protected function schema_index_drop(Schema $schema, string $table_name, string $index_name, bool $safe_check = false): void { $table = $schema->getTable($table_name); + $index_name = self::normalize_index_name($table_name, $index_name); if ($safe_check && !$table->hasIndex($index_name)) { diff --git a/phpBB/phpbb/db/tools/tools_interface.php b/phpBB/phpbb/db/tools/tools_interface.php index 0b8cefd8dd..a3fdae8004 100644 --- a/phpBB/phpbb/db/tools/tools_interface.php +++ b/phpBB/phpbb/db/tools/tools_interface.php @@ -165,6 +165,17 @@ interface tools_interface */ public function sql_create_index(string $table_name, string $index_name, $column); + /** + * Rename index + * + * @param string $table_name Table to modify + * @param string $index_name_old Name of the index to rename + * @param string $index_name_new New name of the index being renamed + * + * @return bool|string[] True if the statements have been executed + */ + public function sql_rename_index(string $table_name, string $index_name_old, string $index_name_new); + /** * Drop Index * @@ -215,4 +226,41 @@ interface tools_interface * @return void */ public function sql_truncate_table(string $table_name): void; + + /** + * Gets current Doctrine DBAL connection + * + * @return \Doctrine\DBAL\Connection + */ + public function get_connection(): \Doctrine\DBAL\Connection; + + /** + * Adds short table name prefix to the index name if needed + * + * @param string $table_name Table name with tables prefix + * @param string $index_name Index name + * @param bool $remove_prefix Flag indicating to remove short table name prefix if exists + * + * @return string Prefixed index name + */ + public static function normalize_index_name(string $table_name, string $index_name, bool $remove_prefix = false): string; + + /** + * Removes prefix from string if exists + * + * @param string $name String to remove the prefix from + * + * @return string Prefixless string + */ + public static function remove_prefix(string $name): string; + + /** + * Tests if a string is prefixed with the prefix defined + * + * @param string $name String to test vs prefix + * @param string $prefix Prefix name + * + * @return bool True if a string id prefixed with the prefix defined, false otherwise + */ + public static function is_prefixed(string $name, string $prefix): bool; } diff --git a/phpBB/phpbb/install/installer.php b/phpBB/phpbb/install/installer.php index 23ab5df919..3664b39f2d 100644 --- a/phpBB/phpbb/install/installer.php +++ b/phpBB/phpbb/install/installer.php @@ -280,7 +280,9 @@ class installer } catch (\Exception $e) { - $this->iohandler->add_error_message($e->getMessage()); + $stack_trace = phpbb_filter_root_path(str_replace("\n", '
', $e->getTraceAsString())); + $message = $e->getMessage(); + $this->iohandler->add_error_message($message, $stack_trace); $this->iohandler->send_response(true); $fail_cleanup = true; } diff --git a/tests/dbal/db_tools_test.php b/tests/dbal/db_tools_test.php index 73543ed97d..42ce4e924b 100644 --- a/tests/dbal/db_tools_test.php +++ b/tests/dbal/db_tools_test.php @@ -232,23 +232,25 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case public function test_column_change_with_index() { + $short_table_name = \phpbb\db\doctrine\table_helper::generate_shortname('table_name'); + // Create column $this->assertFalse($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012')); $this->assertTrue($this->tools->sql_column_add('prefix_table_name', 'c_bug_12012', array('DECIMAL', 0))); $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012')); // Create index over the column - $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012')); + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_i_bug_12012')); $this->assertTrue($this->tools->sql_create_index('prefix_table_name', 'i_bug_12012', array('c_bug_12012', 'c_bool'))); - $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012')); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_i_bug_12012')); // Change type from int to string $this->assertTrue($this->tools->sql_column_change('prefix_table_name', 'c_bug_12012', array('VCHAR:100', ''))); // Remove the index - $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012')); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_i_bug_12012')); $this->assertTrue($this->tools->sql_index_drop('prefix_table_name', 'i_bug_12012')); - $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', 'i_bug_12012')); + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_i_bug_12012')); // Remove the column $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012')); @@ -301,19 +303,21 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case public function test_column_remove_with_index() { + $short_table_name = \phpbb\db\doctrine\table_helper::generate_shortname('table_name'); + // Create column $this->assertFalse($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012_2')); $this->assertTrue($this->tools->sql_column_add('prefix_table_name', 'c_bug_12012_2', array('UINT', 4))); $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012_2')); // Create index over the column - $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', 'bug_12012_2')); + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_bug_12012_2')); $this->assertTrue($this->tools->sql_create_index('prefix_table_name', 'bug_12012_2', array('c_bug_12012_2', 'c_bool'))); - $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'bug_12012_2')); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_bug_12012_2')); - $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', 'bug_12012_3')); + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_bug_12012_3')); $this->assertTrue($this->tools->sql_create_index('prefix_table_name', 'bug_12012_3', array('c_bug_12012_2'))); - $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'bug_12012_3')); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_bug_12012_3')); // Remove the column $this->assertTrue($this->tools->sql_column_exists('prefix_table_name', 'c_bug_12012_2')); @@ -443,24 +447,24 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case public function test_index_exists() { - $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'i_simple')); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', \phpbb\db\doctrine\table_helper::generate_shortname('table_name') . '_i_simple')); } public function test_unique_index_exists() { - $this->assertTrue($this->tools->sql_unique_index_exists('prefix_table_name', 'i_uniq')); + $this->assertTrue($this->tools->sql_unique_index_exists('prefix_table_name', \phpbb\db\doctrine\table_helper::generate_shortname('table_name') . '_i_uniq')); } public function test_create_index_against_index_exists() { $this->tools->sql_create_index('prefix_table_name', 'fookey', array('c_timestamp', 'c_decimal')); - $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', 'fookey')); + $this->assertTrue($this->tools->sql_index_exists('prefix_table_name', \phpbb\db\doctrine\table_helper::generate_shortname('table_name') . '_fookey')); } public function test_create_unique_index_against_unique_index_exists() { $this->tools->sql_create_unique_index('prefix_table_name', 'i_uniq_ts_id', array('c_timestamp', 'c_id')); - $this->assertTrue($this->tools->sql_unique_index_exists('prefix_table_name', 'i_uniq_ts_id')); + $this->assertTrue($this->tools->sql_unique_index_exists('prefix_table_name', \phpbb\db\doctrine\table_helper::generate_shortname('table_name') . '_i_uniq_ts_id')); } public function test_create_int_default_null() @@ -493,27 +497,28 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case $table_suffix = str_repeat('a', 25 - strlen($table_prefix)); $table_name = $table_prefix . $table_suffix; + $short_table_name = \phpbb\db\doctrine\table_helper::generate_shortname($table_suffix); $this->tools->sql_create_table($table_name, $this->table_data); // Index name and table suffix and table prefix have > maximum index length chars in total. // Index name and table suffix have <= maximum index length chars in total. $long_index_name = str_repeat('i', $max_index_length - strlen($table_suffix)); - $this->assertFalse($this->tools->sql_index_exists($table_name, $long_index_name)); + $this->assertFalse($this->tools->sql_index_exists($table_name, $short_table_name . '_' . $long_index_name)); $this->assertTrue($this->tools->sql_create_index($table_name, $long_index_name, array('c_timestamp'))); - $this->assertTrue($this->tools->sql_index_exists($table_name, $long_index_name)); + $this->assertTrue($this->tools->sql_index_exists($table_name, $short_table_name . '_' . $long_index_name)); // Index name and table suffix have > maximum index length chars in total. $very_long_index_name = str_repeat('i', $max_index_length); - $this->assertFalse($this->tools->sql_index_exists($table_name, $very_long_index_name)); + $this->assertFalse($this->tools->sql_index_exists($table_name, $short_table_name . '_' . $very_long_index_name)); $this->assertTrue($this->tools->sql_create_index($table_name, $very_long_index_name, array('c_timestamp'))); - $this->assertTrue($this->tools->sql_index_exists($table_name, $very_long_index_name)); + $this->assertTrue($this->tools->sql_index_exists($table_name, $short_table_name . '_' . $very_long_index_name)); $this->tools->sql_table_drop($table_name); // Index name has > maximum index length chars - that should not be possible. $too_long_index_name = str_repeat('i', $max_index_length + 1); - $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', $too_long_index_name)); + $this->assertFalse($this->tools->sql_index_exists('prefix_table_name', $short_table_name . '_' . $too_long_index_name)); $this->setExpectedTriggerError(E_USER_ERROR); // TODO: Do we want to keep this limitation, if yes reimplement the user check /* https://github.com/phpbb/phpbb/blob/aee5e373bca6cd20d44b99585d3b758276a2d7e6/phpBB/phpbb/db/tools/tools.php#L1488-L1517 */ $this->tools->sql_create_index('prefix_table_name', $too_long_index_name, array('c_timestamp')); diff --git a/tests/dbal/migration/schema_index.php b/tests/dbal/migration/schema_index.php new file mode 100644 index 0000000000..1ea8eb7c78 --- /dev/null +++ b/tests/dbal/migration/schema_index.php @@ -0,0 +1,65 @@ + +* @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_dbal_migration_schema_index extends \phpbb\db\migration\migration +{ + function update_schema() + { + return [ + 'add_tables' => [ + $this->table_prefix . 'foobar1' => [ + 'COLUMNS' => [ + 'user_id' => ['UINT', 0], + 'username' => ['VCHAR:50', 0], + ], + 'KEYS' => [ + 'tstidx_user_id' => ['UNIQUE', 'user_id'], + 'tstidx_username' => ['INDEX', 'username'], + ], + ], + $this->table_prefix . 'foobar2' => [ + 'COLUMNS' => [ + 'ban_userid' => ['ULINT', 0], + 'ban_ip' => ['VCHAR:40', ''], + 'ban_reason' => ['VCHAR:100', ''], + ], + 'KEYS' => [ + 'tstidx_ban_userid' => ['UNIQUE', 'ban_userid'], + 'tstidxban_data' => ['INDEX', ['ban_ip', 'ban_reason']], + ], + ], + ], + + 'rename_index' => [ + $this->table_prefix . 'foobar1' => [ + 'tstidx_user_id' => 'fbr1_user_id', + 'tstidx_username' => 'fbr1_username', + ], + $this->table_prefix . 'foobar2' => [ + 'tstidx_ban_userid' => 'fbr2_ban_userid', + 'tstidxban_data' => 'fbr2_ban_data', + ], + ], + ]; + } + + function revert_schema() + { + return [ + 'drop_tables' => [ + $this->table_prefix . 'foobar1', + $this->table_prefix . 'foobar2', + ], + ]; + } +} diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php index 5d16fa63ee..4207e388de 100644 --- a/tests/dbal/migrator_test.php +++ b/tests/dbal/migrator_test.php @@ -24,6 +24,7 @@ require_once __DIR__ . '/migration/revert_table_with_dependency.php'; 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'; class phpbb_dbal_migrator_test extends phpbb_database_test_case { @@ -416,4 +417,86 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case $this->assertFalse($this->db_tools->sql_column_exists('phpbb_config', 'test_column1')); $this->assertFalse($this->db_tools->sql_table_exists('phpbb_foobar')); } + + public function test_rename_index() + { + $this->migrator->set_migrations(array('phpbb_dbal_migration_schema_index')); + + while (!$this->migrator->finished()) + { + $this->migrator->update(); + } + + $this->assertTrue($this->db_tools->sql_unique_index_exists('phpbb_foobar1', 'fbr1_user_id')); + $this->assertTrue($this->db_tools->sql_index_exists('phpbb_foobar1', 'fbr1_username')); + $this->assertTrue($this->db_tools->sql_unique_index_exists('phpbb_foobar2', 'fbr2_ban_userid')); + $this->assertTrue($this->db_tools->sql_index_exists('phpbb_foobar2', 'fbr2_ban_data')); + + while ($this->migrator->migration_state('phpbb_dbal_migration_schema_index')) + { + $this->migrator->revert('phpbb_dbal_migration_schema_index'); + } + + $this->assertFalse($this->db_tools->sql_table_exists('phpbb_foobar1')); + $this->assertFalse($this->db_tools->sql_table_exists('phpbb_foobar2')); + } + + public function test_schema_generator(): array + { + global $phpbb_root_path, $phpEx; + + $finder_factory = new \phpbb\finder\factory(null, false, $phpbb_root_path, $phpEx); + $finder = $finder_factory->get(); + $migrator_classes = $finder->core_path('phpbb/db/migration/data/')->get_classes(); + + $schema_generator = new \phpbb\db\migration\schema_generator( + $migrator_classes, + $this->config, + $this->db, + $this->db_tools, + $phpbb_root_path, + $phpEx, + 'phpbb_', + self::get_core_tables() + ); + $db_table_schema = $schema_generator->get_schema(); + + $this->assertNotEmpty($db_table_schema); + + return $db_table_schema; + } + + /** + * @depends test_schema_generator + */ + public function test_table_indexes(array $db_table_schema) + { + $table_keys = []; + foreach ($db_table_schema as $table_name => $table_data) + { + if (isset($table_data['KEYS'])) + { + foreach ($table_data['KEYS'] as $key_name => $key_data) + { + $table_keys[$table_name][] = $key_name; + } + } + } + + $this->assertNotEmpty($table_keys); + + $table_names = array_merge(array_keys($db_table_schema), ['phpbb_custom_table']); + $short_table_names = \phpbb\db\doctrine\table_helper::map_short_table_names($table_names, 'phpbb_'); + $this->assertEquals('phpbb_custom_table', array_search(\phpbb\db\doctrine\table_helper::generate_shortname('custom_table'), $short_table_names)); + $this->assertEquals($short_table_names['phpbb_custom_table'], \phpbb\db\doctrine\table_helper::generate_shortname('custom_table')); + + foreach ($table_keys as $table_name => $key_names) + { + $index_prefix = $short_table_names[$table_name] . '_'; + foreach ($key_names as $key_name) + { + $this->assertEquals(0, strpos($key_name, $index_prefix), "$key_name does not contain $index_prefix"); + } + } + } }