1
0
mirror of https://github.com/phpbb/phpbb.git synced 2025-07-22 01:21:23 +02:00

Merge pull request #1995 from EXreaction/ticket/11880

Schema changes can take too long and cause a timeout
This commit is contained in:
Nils Adermann
2014-02-05 17:52:17 +01:00
9 changed files with 344 additions and 26 deletions

View File

@@ -10,6 +10,10 @@ services:
- %core.php_ext%
- %core.table_prefix%
- @migrator.tool_collection
- @migrator.helper
migrator.helper:
class: phpbb\db\migration\helper
migrator.tool_collection:
class: phpbb\di\service_collection

View File

@@ -0,0 +1,82 @@
<?php
/**
*
* @package db
* @copyright (c) 2014 phpBB Group
* @license http://opensource.org/licenses/gpl-license.php GNU Public License
*
*/
namespace phpbb\db\migration;
/**
* The migrator is responsible for applying new migrations in the correct order.
*
* @package db
*/
class helper
{
/**
* Get the schema steps from an array of schema changes
*
* This splits up $schema_changes into individual changes so that the
* changes can be chunked
*
* @param array $schema_changes from migration
* @return array
*/
public function get_schema_steps($schema_changes)
{
$steps = array();
// Nested level of data (only supports 1/2 currently)
$nested_level = array(
'drop_tables' => 1,
'add_tables' => 1,
'change_columns' => 2,
'add_columns' => 2,
'drop_keys' => 2,
'drop_columns' => 2,
'add_primary_keys' => 2, // perform_schema_changes only uses one level, but second is in the function
'add_unique_index' => 2,
'add_index' => 2,
);
foreach ($nested_level as $change_type => $data_depth)
{
if (!empty($schema_changes[$change_type]))
{
foreach ($schema_changes[$change_type] as $key => $value)
{
if ($data_depth === 1)
{
$steps[] = array(
'dbtools.perform_schema_changes', array(array(
$change_type => array(
$value,
),
)),
);
}
else if ($data_depth === 2)
{
foreach ($value as $key2 => $value2)
{
$steps[] = array(
'dbtools.perform_schema_changes', array(array(
$change_type => array(
$key => array(
$key2 => $value2,
),
),
)),
);
}
}
}
}
}
return $steps;
}
}

View File

@@ -25,6 +25,9 @@ class migrator
/** @var \phpbb\db\tools */
protected $db_tools;
/** @var \phpbb\db\migration\helper */
protected $helper;
/** @var string */
protected $table_prefix;
@@ -65,11 +68,12 @@ class migrator
/**
* Constructor of the database migrator
*/
public function __construct(\phpbb\config\config $config, \phpbb\db\driver\driver $db, \phpbb\db\tools $db_tools, $migrations_table, $phpbb_root_path, $php_ext, $table_prefix, $tools)
public function __construct(\phpbb\config\config $config, \phpbb\db\driver\driver $db, \phpbb\db\tools $db_tools, $migrations_table, $phpbb_root_path, $php_ext, $table_prefix, $tools, \phpbb\db\migration\helper $helper)
{
$this->config = $config;
$this->db = $db;
$this->db_tools = $db_tools;
$this->helper = $helper;
$this->migrations_table = $migrations_table;
@@ -83,6 +87,8 @@ class migrator
$this->tools[$tool->get_name()] = $tool;
}
$this->tools['dbtools'] = $this->db_tools;
$this->load_migration_state();
}
@@ -229,9 +235,12 @@ class migrator
if (!$state['migration_schema_done'])
{
$this->last_run_migration['task'] = 'apply_schema_changes';
$this->apply_schema_changes($migration->update_schema());
$state['migration_schema_done'] = true;
$this->last_run_migration['task'] = 'process_schema_step';
$steps = $this->helper->get_schema_steps($migration->update_schema());
$result = $this->process_data_step($steps, $state['migration_data_state']);
$state['migration_data_state'] = ($result === true) ? '' : $result;
$state['migration_schema_done'] = ($result === true);
}
else if (!$state['migration_data_done'])
{
@@ -329,32 +338,27 @@ class migrator
$this->set_migration_state($name, $state);
}
else
else if ($state['migration_schema_done'])
{
$this->apply_schema_changes($migration->revert_schema());
$steps = $this->helper->get_schema_steps($migration->revert_schema());
$result = $this->process_data_step($steps, $state['migration_data_state']);
$sql = 'DELETE FROM ' . $this->migrations_table . "
WHERE migration_name = '" . $this->db->sql_escape($name) . "'";
$this->db->sql_query($sql);
$state['migration_data_state'] = ($result === true) ? '' : $result;
$state['migration_schema_done'] = ($result === true) ? false : true;
unset($this->migration_state[$name]);
if (!$state['migration_schema_done'])
{
$sql = 'DELETE FROM ' . $this->migrations_table . "
WHERE migration_name = '" . $this->db->sql_escape($name) . "'";
$this->db->sql_query($sql);
unset($this->migration_state[$name]);
}
}
return true;
}
/**
* Apply schema changes from a migration
*
* Just calls db_tools->perform_schema_changes
*
* @param array $schema_changes from migration
*/
protected function apply_schema_changes($schema_changes)
{
$this->db_tools->perform_schema_changes($schema_changes);
}
/**
* Process the data step of the migration
*
@@ -498,6 +502,7 @@ class migrator
return $this->get_callable_from_step($step);
break;
case 'custom':
if (!is_callable($parameters[0]))
{

View File

@@ -0,0 +1,33 @@
<?php
/**
*
* @package testing
* @copyright (c) 2014 phpBB Group
* @license http://opensource.org/licenses/gpl-license.php GNU Public License
*
*/
class phpbb_dbal_migration_schema extends \phpbb\db\migration\migration
{
function update_schema()
{
return array(
'add_columns' => array(
$this->table_prefix . 'config' => array(
'test_column1' => array('BOOL', 1),
),
),
);
}
function revert_schema()
{
return array(
'drop_columns' => array(
$this->table_prefix . 'config' => array(
'test_column1',
),
),
);
}
}

View File

@@ -16,6 +16,7 @@ require_once dirname(__FILE__) . '/migration/revert.php';
require_once dirname(__FILE__) . '/migration/revert_with_dependency.php';
require_once dirname(__FILE__) . '/migration/fail.php';
require_once dirname(__FILE__) . '/migration/installed.php';
require_once dirname(__FILE__) . '/migration/schema.php';
class phpbb_dbal_migrator_test extends phpbb_database_test_case
{
@@ -49,7 +50,8 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
dirname(__FILE__) . '/../../phpBB/',
'php',
'phpbb_',
$tools
$tools,
new \phpbb\db\migration\helper()
);
$container = new phpbb_mock_container_builder();
@@ -267,4 +269,23 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
$this->fail('Installed test failed');
}
}
public function test_schema()
{
$this->migrator->set_migrations(array('phpbb_dbal_migration_schema'));
while (!$this->migrator->finished())
{
$this->migrator->update();
}
$this->assertTrue($this->db_tools->sql_column_exists('phpbb_config', 'test_column1'));
while ($this->migrator->migration_state('phpbb_dbal_migration_schema'))
{
$this->migrator->revert('phpbb_dbal_migration_schema');
}
$this->assertFalse($this->db_tools->sql_column_exists('phpbb_config', 'test_column1'));
}
}

View File

@@ -106,7 +106,8 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
$phpbb_root_path,
$php_ext,
$table_prefix,
array()
array(),
new \phpbb\db\migration\helper()
);
$container = new phpbb_mock_container_builder();
$container->set('migrator', $migrator);

View File

@@ -62,7 +62,8 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
$this->phpbb_root_path,
'php',
$this->table_prefix,
array()
array(),
new \phpbb\db\migration\helper()
);
$container = new phpbb_mock_container_builder();
$container->set('migrator', $migrator);

View File

@@ -0,0 +1,170 @@
<?php
/**
*
* @package testing
* @copyright (c) 2014 phpBB Group
* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
*
*/
class get_schema_steps_test extends phpbb_test_case
{
public function setUp()
{
parent::setUp();
$this->helper = new \phpbb\db\migration\helper();
}
public function schema_provider()
{
return array(
array(
array(
'add_tables' => array('table1', 'table2', 'table3'),
'drop_tables' => array('table1', 'table2', 'table3'),
'add_index' => array(
'table1' => array(
'index1' => 'column1',
'index2' => 'column2',
),
'table2' => array(
'index1' => 'column1',
'index2' => 'column2',
),
),
'add_columns' => array(
'table1' => array(
'column1' => array('foo'),
'column2' => array('bar'),
),
),
'change_columns' => array(
'table1' => array(
'column1' => array('foo'),
'column2' => array('bar'),
),
),
'drop_columns' => array(
'table1' => array(
'column1',
'column2',
),
),
'add_unique_index' => array(
'table1' => array(
'index1' => 'column1',
'index2' => 'column2',
),
),
'drop_keys' => array(
'table1' => array(
'column1',
'column2',
),
),
'add_primary_keys' => array(
'table1' => array('foo'),
'table2' => array('bar'),
'table3' => array('foobar'),
),
),
array(
array('dbtools.perform_schema_changes', array(array('drop_tables' => array('table1')))),
array('dbtools.perform_schema_changes', array(array('drop_tables' => array('table2')))),
array('dbtools.perform_schema_changes', array(array('drop_tables' => array('table3')))),
array('dbtools.perform_schema_changes', array(array('add_tables' => array('table1')))),
array('dbtools.perform_schema_changes', array(array('add_tables' => array('table2')))),
array('dbtools.perform_schema_changes', array(array('add_tables' => array('table3')))),
array('dbtools.perform_schema_changes', array(array('change_columns' => array(
'table1' => array(
'column1' => array('foo'),
),
)))),
array('dbtools.perform_schema_changes', array(array('change_columns' => array(
'table1' => array(
'column2' => array('bar'),
),
)))),
array('dbtools.perform_schema_changes', array(array('add_columns' => array(
'table1' => array(
'column1' => array('foo'),
),
)))),
array('dbtools.perform_schema_changes', array(array('add_columns' => array(
'table1' => array(
'column2' => array('bar'),
),
)))),
array('dbtools.perform_schema_changes', array(array('drop_keys' => array(
'table1' => array(
0 => 'column1',
),
)))),
array('dbtools.perform_schema_changes', array(array('drop_keys' => array(
'table1' => array(
1 => 'column2',
),
)))),
array('dbtools.perform_schema_changes', array(array('drop_columns' => array(
'table1' => array(
0 => 'column1',
),
)))),
array('dbtools.perform_schema_changes', array(array('drop_columns' => array(
'table1' => array(
1 => 'column2',
),
)))),
array('dbtools.perform_schema_changes', array(array('add_primary_keys' => array(
'table1' => array('foo'),
)))),
array('dbtools.perform_schema_changes', array(array('add_primary_keys' => array(
'table2' => array('bar'),
)))),
array('dbtools.perform_schema_changes', array(array('add_primary_keys' => array(
'table3' => array('foobar'),
)))),
array('dbtools.perform_schema_changes', array(array('add_unique_index' => array(
'table1' => array(
'index1' => 'column1',
),
)))),
array('dbtools.perform_schema_changes', array(array('add_unique_index' => array(
'table1' => array(
'index2' => 'column2',
),
)))),
array('dbtools.perform_schema_changes', array(array('add_index' => array(
'table1' => array(
'index1' => 'column1',
),
)))),
array('dbtools.perform_schema_changes', array(array('add_index' => array(
'table1' => array(
'index2' => 'column2',
),
)))),
array('dbtools.perform_schema_changes', array(array('add_index' => array(
'table2' => array(
'index1' => 'column1',
),
)))),
array('dbtools.perform_schema_changes', array(array('add_index' => array(
'table2' => array(
'index2' => 'column2',
),
)))),
),
),
);
}
/**
* @dataProvider schema_provider
*/
public function test_get_schema_steps($schema_changes, $expected)
{
$this->assertEquals($expected, $this->helper->get_schema_steps($schema_changes));
}
}

View File

@@ -194,7 +194,8 @@ class phpbb_functional_test_case extends phpbb_test_case
$phpbb_root_path,
$php_ext,
self::$config['table_prefix'],
array()
array(),
new \phpbb\db\migration\helper()
);
$container = new phpbb_mock_container_builder();
$container->set('migrator', $migrator);