mirror/php-phpbb
1
0
Fork 0
mirror of https://github.com/phpbb/phpbb.git synced 2025-08-04 15:57:45 +02:00
Code Issues Releases Wiki Activity

Merge pull request #6830 from rxu/ticket/17525

Browse Source 
[ticket/17525] Correctly handle Doctrine DB tools exceptions, enrich error info
This commit is contained in:
Marc Alexander
2025-07-05 09:31:52 +02:00
committed by GitHub
parent 3cc7076513 aa3f266b8c
commit 4367bc8d11
29 changed files with 621 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
tests/dbal/auto_increment_test.php
View File

@@ -28,10 +28,12 @@ class phpbb_dbal_auto_increment_test extends phpbb_database_test_case
{
parent::setUp();
$table_prefix = 'prefix_';
$this->db = $this->new_dbal();
$this->db_doctrine = $this->new_doctrine_dbal();
$factory = new \phpbb\db\tools\factory();
$this->tools = $factory->get($this->db_doctrine);
$this->tools->set_table_prefix($table_prefix);
$this->table_data = array(
'COLUMNS' => array(

42
tests/dbal/db_tools_test.php
View File

@@ -35,12 +35,15 @@ class phpbb_dbal_db_tools_test extends phpbb_database_test_case
protected function setUp(): void
{
parent::setUp();
$table_prefix = 'prefix_';
$this->db = $this->new_dbal();
$this->doctrine_db = $this->new_doctrine_dbal();
$factory = new \phpbb\db\tools\factory();
$this->tools = $factory->get($this->doctrine_db);
$this->tools->set_table_prefix($table_prefix);
$this->table_data = array(
'COLUMNS' => array(
@@ -232,23 +235,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 +306,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 +450,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 +500,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'));

65
tests/dbal/migration/schema_index.php Normal file
View File

@@ -0,0 +1,65 @@
<?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.
*
*/
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',
],
];
}
}

86
tests/dbal/migrator_test.php
View File

@@ -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
{
@@ -52,12 +53,15 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
protected function setUp(): void
{
global $table_prefix;
parent::setUp();
$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->config = new \phpbb\config\db($this->db, new phpbb_mock_cache, 'phpbb_config');
@@ -416,4 +420,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");
}
}
}
}

3
tests/extension/manager_test.php
View File

@@ -154,6 +154,7 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
{
$phpbb_root_path = __DIR__ . './../../phpBB/';
$php_ext = 'php';
$table_prefix = 'phpbb_';
$config = new \phpbb\config\config(array('version' => PHPBB_VERSION));
$db = $this->new_dbal();
@@ -162,7 +163,7 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
$factory = new \phpbb\db\tools\factory();
$finder_factory = new \phpbb\finder\factory(null, false, $phpbb_root_path, $php_ext);
$db_tools = $factory->get($db_doctrine);
$table_prefix = 'phpbb_';
$db_tools->set_table_prefix($table_prefix);
$container = new phpbb_mock_container_builder();

3
tests/extension/metadata_manager_test.php
View File

@@ -37,6 +37,7 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
{
parent::setUp();
$this->table_prefix = 'phpbb_';
$this->config = new \phpbb\config\config(array(
'version' => '3.1.0',
));
@@ -45,13 +46,13 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
$factory = new \phpbb\db\tools\factory();
$this->db_tools = $factory->get($this->db_doctrine);
$this->db_tools->set_table_prefix($this->table_prefix);
$finder_factory = $this->createMock('\phpbb\finder\factory');
$this->phpbb_root_path = __DIR__ . '/';
$this->phpEx = 'php';
$this->cache = new \phpbb\cache\service(new phpbb_mock_cache(), $this->config, $this->db, $phpbb_dispatcher, $this->phpbb_root_path, $this->phpEx);
$this->table_prefix = 'phpbb_';
$container = new phpbb_mock_container_builder();
$cache_path = $this->phpbb_root_path . 'cache/twig';

8
tests/migrations/migrations_check_config_added_test.php
View File

@@ -55,6 +55,10 @@ class migrations_check_config_added_test extends phpbb_test_case
{
global $phpbb_root_path, $phpEx;
$this->table_prefix = 'phpbb_';
$this->phpbb_root_path = $phpbb_root_path;
$this->php_ext = $phpEx;
$this->config = new \phpbb\config\config([
'search_type' => '\phpbb\search\fulltext_mysql',
]);
@@ -63,9 +67,7 @@ class migrations_check_config_added_test extends phpbb_test_case
$this->db_doctrine = $this->createMock(\Doctrine\DBAL\Connection::class);
$factory = new \phpbb\db\tools\factory();
$this->db_tools = $factory->get($this->db_doctrine);
$this->table_prefix = 'phpbb_';
$this->phpbb_root_path = $phpbb_root_path;
$this->php_ext = $phpEx;
$this->db_tools->set_table_prefix($this->table_prefix);
$tools = [
new \phpbb\db\migration\tool\config($this->config),

4
tests/migrator/get_callable_from_step_test.php
View File

@@ -23,6 +23,8 @@ class get_callable_from_step_test extends phpbb_database_test_case
$db = $this->new_dbal();
$db_doctrine = $this->new_doctrine_dbal();
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($db_doctrine);
$db_tools->set_table_prefix($table_prefix);
$user = $this->getMockBuilder('\phpbb\user')->disableOriginalConstructor()->getMock();
$user->ip = '127.0.0.1';
$module_manager = new \phpbb\module\module_manager(
@@ -38,7 +40,7 @@ class get_callable_from_step_test extends phpbb_database_test_case
new phpbb_mock_container_builder(),
new \phpbb\config\config(array()),
$db,
$factory->get($db_doctrine),
$db_tools,
'phpbb_migrations',
$phpbb_root_path,
$php_ext,

8
tests/migrator/schema_generator_test.php
View File

@@ -30,14 +30,16 @@ class schema_generator_test extends phpbb_test_case
parent::setUp();
$this->table_prefix = 'phpbb_';
$this->phpbb_root_path = $phpbb_root_path;
$this->php_ext = $phpEx;
$this->config = new \phpbb\config\config(array());
$this->db = new \phpbb\db\driver\sqlite3();
$this->doctrine_db = \phpbb\db\doctrine\connection_factory::get_connection(new phpbb_mock_config_php_file());
$factory = new \phpbb\db\tools\factory();
$this->db_tools = $factory->get($this->doctrine_db);
$this->table_prefix = 'phpbb_';
$this->phpbb_root_path = $phpbb_root_path;
$this->php_ext = $phpEx;
$this->db_tools->set_table_prefix($this->table_prefix);
}
protected function get_schema_generator(array $class_names)

1
tests/test_framework/phpbb_database_test_case.php
View File

@@ -88,6 +88,7 @@ abstract class phpbb_database_test_case extends TestCase
$doctrine = \phpbb\db\doctrine\connection_factory::get_connection(new phpbb_mock_config_php_file());
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($doctrine, true);
$db_tools->set_table_prefix($table_prefix);
$schema_generator = new \phpbb\db\migration\schema_generator($classes, new \phpbb\config\config(array()), $db, $db_tools, $phpbb_root_path, $phpEx, $table_prefix, self::get_core_tables());
file_put_contents(self::$schema_file, json_encode($schema_generator->get_schema()));

6
tests/test_framework/phpbb_database_test_connection_manager.php
View File

@@ -327,6 +327,8 @@ class phpbb_database_test_connection_manager
*/
protected function load_schema_from_file($directory, \phpbb\db\driver\driver_interface $db, \Doctrine\DBAL\Connection $doctrine)
{
global $table_prefix;
$schema = $this->dbms['SCHEMA'];
if ($this->config['dbms'] == 'phpbb\db\driver\mysql')
@@ -363,7 +365,7 @@ class phpbb_database_test_connection_manager
}
else
{
global $phpbb_root_path, $phpEx, $table_prefix;
global $phpbb_root_path, $phpEx;
$finder = new \phpbb\finder\finder(null, false, $phpbb_root_path, $phpEx);
$classes = $finder->core_path('phpbb/db/migration/data/')
@@ -373,6 +375,7 @@ class phpbb_database_test_connection_manager
$doctrine = \phpbb\db\doctrine\connection_factory::get_connection(new phpbb_mock_config_php_file());
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($doctrine, true);
$db_tools->set_table_prefix($table_prefix);
$tables = phpbb_database_test_case::get_core_tables();
$schema_generator = new \phpbb\db\migration\schema_generator($classes, new \phpbb\config\config(array()), $db, $db_tools, $phpbb_root_path, $phpEx, $table_prefix, $tables);
@@ -381,6 +384,7 @@ class phpbb_database_test_connection_manager
$factory = new \phpbb\db\tools\factory();
$db_tools = $factory->get($doctrine);
$db_tools->set_table_prefix($table_prefix);
foreach ($db_table_schema as $table_name => $table_data)
{
$db_tools->sql_create_table(

1
tests/test_framework/phpbb_functional_test_case.php
View File

@@ -286,6 +286,7 @@ class phpbb_functional_test_case extends phpbb_test_case
$factory = new \phpbb\db\tools\factory();
$finder_factory = new \phpbb\finder\factory(null, false, $phpbb_root_path, $phpEx);
$db_tools = $factory->get($db_doctrine);
$db_tools->set_table_prefix(self::$config['table_prefix']);
$container = new phpbb_mock_container_builder();
$migrator = new \phpbb\db\migrator(