Compare commits

...

5 commits

Author SHA1 Message Date
rxu
b6ad00e335
Merge 5e2dcf9d8f into b8c49f9711 2025-06-16 18:24:59 +00:00
rxu
5e2dcf9d8f
[ticket/17525] Avoid more index name duplication
PHPBB-17525
2025-06-17 01:24:43 +07:00
rxu
7b0b95250c
[ticket/17525] Avoid more index name duplication
Many more tables have indexes with the same names which can cause issues
on SQLite/Postgres. Also, add an option for migrations to rename indexes.

PHPBB-17525
2025-06-17 00:46:17 +07:00
rxu
0e0214a71d
[ticket/17525] Avoid index name duplication (auth_role_id)
phpbb_acl_groups and phpbb_acl_users have indexes
with the same names (auth_role_id) which can cause issues on SQLite/Postgres

PHPBB-17525
2025-06-15 15:51:59 +07:00
rxu
8d1f8af7c6
[ticket/17525] Correctly handle Doctrine DB tools exceptions, enrich error info
PHPBB-17525
2025-06-15 13:03:55 +07:00
9 changed files with 230 additions and 28 deletions

View file

@ -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", '<br>', $e->getTraceAsString()));
$message = $e->getMessage();
$this->template->assign_var('MIGRATOR_ERROR', '<b>' . $message . '</b><br><br>' . $stack_trace);
}
$this->tpl_name = 'acp_ext_enable';

View file

@ -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);
}

View file

@ -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();
}

View file

@ -0,0 +1,132 @@
<?php
/**
*
* This file is part of the phpBB Forum Software package.
*
* @copyright (c) phpBB Limited <https://www.phpbb.com>
* @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;
class rename_auth_role_id_index extends migration
{
public static function depends_on()
{
return [
'\phpbb\db\migration\data\v400\dev',
];
}
public function update_schema()
{
return [
'rename_index' => [
$this->table_prefix . 'acl_groups' => [
'auth_role_id' => 'aclgrps_auth_role_id',
'group_id' => 'aclgrps_group_id',
],
$this->table_prefix . 'acl_users' => [
'auth_role_id' => 'aclusrs_auth_role_id',
'user_id' => 'aclusrs_user_id',
],
$this->table_prefix . 'attachments' => [
'poster_id' => 'attchmnts_poster_id',
'topic_id' => 'attchmnts_topic_id',
],
$this->table_prefix . 'forums' => [
'left_right_id' => 'frms_left_right_id',
],
$this->table_prefix . 'forums_watch' => [
'forum_id' => 'frmswtch_forum_id',
'user_id' => 'frmswtch_user_id',
],
$this->table_prefix . 'log' => [
'forum_id' => 'log_forum_id',
'topic_id' => 'log_topic_id',
'user_id' => 'log_user_id',
],
$this->table_prefix . 'login_attempts' => [
'user_id' => 'lgnatmpts_user_id',
],
$this->table_prefix . 'moderator_cache' => [
'forum_id' => 'mdrtrcch_forum_id',
],
$this->table_prefix . 'modules' => [
'left_right_id' => 'mdls_left_right_id',
],
$this->table_prefix . 'oauth_states' => [
'provider' => 'oauthsts_provider',
'user_id' => 'oauthsts_user_id',
],
$this->table_prefix . 'oauth_tokens' => [
'provider' => 'oauthtkns_provider',
'user_id' => 'oauthtkns_user_id',
],
$this->table_prefix . 'poll_options' => [
'topic_id' => 'pllopts_topic_id',
],
$this->table_prefix . 'poll_votes' => [
'topic_id' => 'pllvts_topic_id',
],
$this->table_prefix . 'posts' => [
'forum_id' => 'psts_forum_id',
'poster_id' => 'psts_poster_id',
'topic_id' => 'psts_topic_id',
],
$this->table_prefix . 'privmsgs' => [
'author_id' => 'pms_author_id',
],
$this->table_prefix . 'privmsgs_folder' => [
'user_id' => 'pmsfldr_user_id',
],
$this->table_prefix . 'privmsgs_rules' => [
'user_id' => 'pmsrls_user_id',
],
$this->table_prefix . 'privmsgs_to' => [
'author_id' => 'pmsto_author_id',
],
$this->table_prefix . 'reports' => [
'post_id' => 'rprts_post_id',
],
$this->table_prefix . 'search_wordmatch' => [
'post_id' => 'wrdmtch_post_id',
],
$this->table_prefix . 'topics' => [
'forum_id' => 'tpcs_forum_id',
],
$this->table_prefix . 'topics_track' => [
'forum_id' => 'tpcstrk_forum_id',
'topic_id' => 'tpcstrk_topic_id',
],
$this->table_prefix . 'topics_watch' => [
'topic_id' => 'tpcswtch_topic_id',
'user_id' => 'tpcswtch_user_id',
],
$this->table_prefix . 'user_group' => [
'group_id' => 'usrgrp_group_id',
'user_id' => 'usrgrp_user_id',
],
$this->table_prefix . 'user_notifications' => [
'user_id' => 'usrntf_user_id',
],
],
];
}
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;
}
}

View file

@ -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)

View file

@ -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];
};

View file

@ -330,6 +330,19 @@ class doctrine implements tools_interface
);
}
/**
* {@inheritDoc}
*/
public function sql_rename_index(string $table_name, string $index_name_old, string $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}
*/
@ -458,34 +471,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 +558,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)
@ -839,6 +849,27 @@ 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);
if ($safe_check && !$table->hasIndex($index_name_old))
{
return;
}
$table->renameIndex($index_name_old, $index_name_new);
}
/**
* @param Schema $schema
* @param string $table_name

View file

@ -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
*

View file

@ -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", '<br>', $e->getTraceAsString()));
$message = $e->getMessage();
$this->iohandler->add_error_message($message, $stack_trace);
$this->iohandler->send_response(true);
$fail_cleanup = true;
}