makary/phpbb
1
0
Fork 0
mirror of https://github.com/phpbb/phpbb.git synced 2025-07-09 11:38:52 +00:00
Code Issues Projects Releases Packages Wiki Activity

[ticket/17525] Address the PR review comments

Browse source 
PHPBB-17525
This commit is contained in:
rxu 2025-07-05 12:27:10 +07:00
parent 10921ebc58
commit aa3f266b8c
No known key found for this signature in database
GPG key ID: 955F0567380E586A
13 changed files with 48 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
phpBB/adm/style/acp_ext_delete_data.html
View file

@ -11,6 +11,7 @@
<div class="errorbox">
<p><strong>{{ lang('MIGRATION_EXCEPTION_ERROR') }}</strong></p>
<p>{{ MIGRATOR_ERROR }}</p>
{% if MIGRATOR_ERROR_STACK_TRACE %}<p>{{ MIGRATOR_ERROR_STACK_TRACE|nl2br }}</p>{% endif %}
<p><a href="{{ U_RETURN }}">{{ lang('RETURN_TO_EXTENSION_LIST') }}</a></p>
</div>
{% elseif S_PRE_STEP %}

1
phpBB/adm/style/acp_ext_enable.html
View file

@ -11,6 +11,7 @@
<div class="errorbox">
<p><strong>{{ lang('MIGRATION_EXCEPTION_ERROR') }}</strong></p>
<p>{{ MIGRATOR_ERROR }}</p>
{% if MIGRATOR_ERROR_STACK_TRACE %}<p>{{ MIGRATOR_ERROR_STACK_TRACE|nl2br }}</p>{% endif %}
<p><a href="{{ U_RETURN }}">{{ lang('RETURN_TO_EXTENSION_LIST') }}</a></p>
</div>
{% elseif S_PRE_STEP %}

10
phpBB/includes/acp/acp_extensions.php
View file

@ -278,9 +278,8 @@ class acp_extensions
}
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->template->assign_var('MIGRATOR_ERROR', $e->getMessage());
$this->template->assign_var('MIGRATOR_ERROR_STACK_TRACE', phpbb_filter_root_path($e->getTraceAsString()));
}
$this->tpl_name = 'acp_ext_enable';
@ -369,6 +368,11 @@ class acp_extensions
{
$this->template->assign_var('MIGRATOR_ERROR', $e->getLocalisedMessage($this->user));
}
catch (\Exception $e)
{
$this->template->assign_var('MIGRATOR_ERROR', $e->getMessage());
$this->template->assign_var('MIGRATOR_ERROR_STACK_TRACE', phpbb_filter_root_path($e->getTraceAsString()));
}
$this->tpl_name = 'acp_ext_delete_data';

2
phpBB/phpbb/db/doctrine/table_helper.php
View file

@ -125,7 +125,7 @@ class table_helper
}
/**
* Maps table names to thair short names for the purpose of prefixing tables' index names.
* Maps table names to their 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.

2
phpBB/phpbb/db/migration/data/v400/rename_duplicated_index_names.php
View file

@ -52,7 +52,7 @@ class rename_duplicated_index_names extends migration
foreach ($key_names as $key_name)
{
// If 'old' key name is already new format, do not rename it
if (doctrine_dbtools::is_prefixed($key_name, $short_table_names[$table_name]))
if (str_starts_with($key_name, $short_table_names[$table_name]))
{
continue;
}

46
phpBB/phpbb/db/tools/doctrine.php
View file

@ -423,7 +423,7 @@ class doctrine implements tools_interface
*/
public static function add_prefix(string $name, string $prefix): string
{
return strpos($prefix, '_', -1) ? $prefix . $name : $prefix . '_' . $name;
return str_ends_with($prefix, '_') ? $prefix . $name : $prefix . '_' . $name;
}
/**
@ -431,16 +431,8 @@ class doctrine implements tools_interface
*/
public static function remove_prefix(string $name, string $prefix = ''): string
{
$prefix = strpos($prefix, '_', -1) ? $prefix : $prefix . '_';
return $prefix && self::is_prefixed($name, $prefix) ? substr($name, strlen($prefix)) : $name;
}
/**
* {@inheritDoc}
*/
public static function is_prefixed(string $name, string $prefix): bool
{
return strpos($name, $prefix) === 0;
$prefix = str_ends_with($prefix, '_') ? $prefix : $prefix . '_';
return $prefix && str_starts_with($name, $prefix) ? substr($name, strlen($prefix)) : $name;
}
/**
@ -692,7 +684,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::is_prefixed($key_name, $short_table_name) ? self::add_prefix($key_name, $short_table_name) : $key_name;
$key_name = !str_starts_with($key_name, $short_table_name) ? self::add_prefix($key_name, $short_table_name) : $key_name;
// Supports key columns defined with there length
$columns = array_map(function (string $column)
@ -725,6 +717,8 @@ class doctrine implements tools_interface
}
/**
* Removes a table
*
* @param Schema $schema
* @param string $table_name
* @param bool $safe_check
@ -742,6 +736,8 @@ class doctrine implements tools_interface
}
/**
* Adds column to a table
*
* @param Schema $schema
* @param string $table_name
* @param string $column_name
@ -772,6 +768,8 @@ class doctrine implements tools_interface
}
/**
* Alters column properties
*
* @param Schema $schema
* @param string $table_name
* @param string $column_name
@ -802,6 +800,8 @@ class doctrine implements tools_interface
}
/**
* Alters column properties or adds a column
*
* @param Schema $schema
* @param string $table_name
* @param string $column_name
@ -824,6 +824,8 @@ class doctrine implements tools_interface
}
/**
* Removes a column in a table
*
* @param Schema $schema
* @param string $table_name
* @param string $column_name
@ -876,6 +878,8 @@ class doctrine implements tools_interface
}
/**
* Creates non-unique index for a table
*
* @param Schema $schema
* @param string $table_name
* @param string $index_name
@ -889,7 +893,7 @@ class doctrine implements tools_interface
$columns = (is_array($column)) ? $column : [$column];
$table = $schema->getTable($table_name);
$short_table_name = table_helper::generate_shortname(self::remove_prefix($table_name, $this->table_prefix));
$index_name = !self::is_prefixed($index_name, $short_table_name) ? self::add_prefix($index_name, $short_table_name) : $index_name;
$index_name = !str_starts_with($index_name, $short_table_name) ? self::add_prefix($index_name, $short_table_name) : $index_name;
if ($safe_check && $table->hasIndex($index_name))
{
@ -900,6 +904,8 @@ class doctrine implements tools_interface
}
/**
* Renames table index
*
* @param Schema $schema
* @param string $table_name
* @param string $index_name_old
@ -915,9 +921,9 @@ class doctrine implements tools_interface
if (!$table->hasIndex($index_name_old))
{
$index_name_old = !self::is_prefixed($index_name_old, $short_table_name) ? self::add_prefix($index_name_old, $short_table_name) : self::remove_prefix($index_name_old, $short_table_name);
$index_name_old = !str_starts_with($index_name_old, $short_table_name) ? self::add_prefix($index_name_old, $short_table_name) : self::remove_prefix($index_name_old, $short_table_name);
}
$index_name_new = !self::is_prefixed($index_name_new, $short_table_name) ? self::add_prefix($index_name_new, $short_table_name) : $index_name_new;
$index_name_new = !str_starts_with($index_name_new, $short_table_name) ? self::add_prefix($index_name_new, $short_table_name) : $index_name_new;
if ($safe_check && !$table->hasIndex($index_name_old))
{
@ -928,6 +934,8 @@ class doctrine implements tools_interface
}
/**
* Creates unique (non-primary) index for a table
*
* @param Schema $schema
* @param string $table_name
* @param string $index_name
@ -941,7 +949,7 @@ class doctrine implements tools_interface
$columns = (is_array($column)) ? $column : [$column];
$table = $schema->getTable($table_name);
$short_table_name = table_helper::generate_shortname(self::remove_prefix($table_name, $this->table_prefix));
$index_name = !self::is_prefixed($index_name, $short_table_name) ? self::add_prefix($index_name, $short_table_name) : $index_name;
$index_name = !str_starts_with($index_name, $short_table_name) ? self::add_prefix($index_name, $short_table_name) : $index_name;
if ($safe_check && $table->hasIndex($index_name))
{
@ -952,6 +960,8 @@ class doctrine implements tools_interface
}
/**
* Removes table index
*
* @param Schema $schema
* @param string $table_name
* @param string $index_name
@ -966,7 +976,7 @@ class doctrine implements tools_interface
if (!$table->hasIndex($index_name))
{
$index_name = !self::is_prefixed($index_name, $short_table_name) ? self::add_prefix($index_name, $short_table_name) : self::remove_prefix($index_name, $short_table_name);
$index_name = !str_starts_with($index_name, $short_table_name) ? self::add_prefix($index_name, $short_table_name) : self::remove_prefix($index_name, $short_table_name);
}
if ($safe_check && !$table->hasIndex($index_name))
@ -978,6 +988,8 @@ class doctrine implements tools_interface
}
/**
* Creates primary key for a table
*
* @param $column
* @param Schema $schema
* @param string $table_name

10
phpBB/phpbb/db/tools/tools_interface.php
View file

@ -255,16 +255,6 @@ interface tools_interface
*/
public static function remove_prefix(string $name, string $prefix = ''): 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;
/**
* Sets table prefix
*

6
tests/captcha/qa_test.php
View file

@ -25,7 +25,7 @@ class phpbb_captcha_qa_test extends \phpbb_database_test_case
protected function setUp(): void
{
global $db, $request, $phpbb_container, $table_prefix;
global $db, $request, $phpbb_container;
$db = $this->new_dbal();
$db_doctrine = $this->new_doctrine_dbal();
@ -35,9 +35,7 @@ class phpbb_captcha_qa_test extends \phpbb_database_test_case
$request = new \phpbb_mock_request();
$phpbb_container = new \phpbb_mock_container_builder();
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($db_doctrine);
$db_tools->set_table_prefix($table_prefix);
$phpbb_container->set('dbal.tools', $db_tools);
$phpbb_container->set('dbal.tools', $factory->get($db_doctrine));
$this->qa = new \phpbb\captcha\plugins\qa('phpbb_captcha_questions', 'phpbb_captcha_answers', 'phpbb_qa_confirm');
}

3
tests/console/user/base.php
View file

@ -32,7 +32,7 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
protected function setUp(): void
{
global $auth, $db, $cache, $config, $user, $phpbb_dispatcher, $phpbb_container, $phpbb_root_path, $phpEx, $table_prefix;
global $auth, $db, $cache, $config, $user, $phpbb_dispatcher, $phpbb_container, $phpbb_root_path, $phpEx;
$phpbb_dispatcher = new \phpbb\event\dispatcher();
$phpbb_container = new phpbb_mock_container_builder();
@ -141,7 +141,6 @@ abstract class phpbb_console_user_base extends phpbb_database_test_case
$factory = new \phpbb\db\tools\factory();
$db_doctrine = $this->new_doctrine_dbal();
$db_tools = $factory->get($db_doctrine);
$db_tools->set_table_prefix($table_prefix);
$migrator = new \phpbb\db\migrator(
$phpbb_container,
$config,

3
tests/dbal/auto_increment_test.php
View file

@ -26,10 +26,9 @@ class phpbb_dbal_auto_increment_test extends phpbb_database_test_case
protected function setUp(): void
{
global $table_prefix;
parent::setUp();
$table_prefix = 'prefix_';
$this->db = $this->new_dbal();
$this->db_doctrine = $this->new_doctrine_dbal();
$factory = new \phpbb\db\tools\factory();

12
tests/migrator/convert_timezones_test.php
View file

@ -18,13 +18,10 @@ class phpbb_migrator_convert_timezones_test extends phpbb_database_test_case
public function getDataSet()
{
global $table_prefix;
$this->db = $this->new_dbal();
$this->db_doctrine = $this->new_doctrine_dbal();
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($this->db_doctrine);
$db_tools->set_table_prefix($table_prefix);
// user_dst doesn't exist anymore, must re-add it to test this
$db_tools->sql_column_add('phpbb_users', 'user_dst', array('BOOL', 1));
@ -58,18 +55,16 @@ class phpbb_migrator_convert_timezones_test extends phpbb_database_test_case
{
parent::setUp();
global $phpbb_root_path, $phpEx, $table_prefix;
global $phpbb_root_path, $phpEx;
$this->db = $this->new_dbal();
$this->db_doctrine = $this->new_doctrine_dbal();
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($this->db_doctrine);
$db_tools->set_table_prefix($table_prefix);
$this->migration = new \phpbb\db\migration\data\v310\timezone(
new \phpbb\config\config(array()),
$this->db,
$db_tools,
$factory->get($this->db_doctrine),
$phpbb_root_path,
$phpEx,
'phpbb_',
@ -89,8 +84,6 @@ class phpbb_migrator_convert_timezones_test extends phpbb_database_test_case
public function test_convert()
{
global $table_prefix;
$this->migration->update_timezones(0);
$sql = 'SELECT user_id, user_timezone
@ -105,7 +98,6 @@ class phpbb_migrator_convert_timezones_test extends phpbb_database_test_case
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($this->db_doctrine);
$db_tools->set_table_prefix($table_prefix);
// Remove the user_dst field again
$db_tools->sql_column_remove('phpbb_users', 'user_dst');

3
tests/notification/convert_test.php
View file

@ -25,13 +25,12 @@ class phpbb_notification_convert_test extends phpbb_database_test_case
{
parent::setUp();
global $phpbb_root_path, $phpEx, $table_prefix;
global $phpbb_root_path, $phpEx;
$this->db = $this->new_dbal();
$this->doctrine_db = $this->new_doctrine_dbal();
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($this->doctrine_db);
$db_tools->set_table_prefix($table_prefix);
$core_tables = self::get_core_tables();
// Add user_notify_type column for testing this migration and set type

3
tests/search/native_test.php
View file

@ -25,7 +25,7 @@ class phpbb_search_native_test extends phpbb_search_test_case
protected function setUp(): void
{
global $phpbb_root_path, $phpEx, $config, $cache, $table_prefix;
global $phpbb_root_path, $phpEx, $config, $cache;
parent::setUp();
@ -41,7 +41,6 @@ class phpbb_search_native_test extends phpbb_search_test_case
$this->db = $this->new_dbal();
$tools_factory = new \phpbb\db\tools\factory();
$this->db_tools = $tools_factory->get($this->new_doctrine_dbal());
$this->db_tools->set_table_prefix($table_prefix);
$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
$class = self::get_search_wrapper('\phpbb\search\backend\fulltext_native');
$config['fulltext_native_min_chars'] = 2;