[ticket/16668] Refactor schema_generator

PHPBB3-16668
This commit is contained in:
Máté Bartus 2020-12-30 18:25:31 +01:00
parent 16d48a5e08
commit 71441e3fb0
9 changed files with 343 additions and 163 deletions

View file

@ -29,6 +29,7 @@
"ext-json": "*",
"ext-mbstring": "*",
"bantu/ini-get-wrapper": "~1.0",
"chita/topological_sort": "^1.0",
"composer/composer": "^2.0",
"composer/installers": "^1.9",
"composer/package-versions-deprecated": "^1.11",

48
phpBB/composer.lock generated
View file

@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
"This file is @generated automatically"
],
"content-hash": "b38b54a0e735586afaf2788716cc2535",
"content-hash": "569d46cb51aed67c281b0425c2fc0381",
"packages": [
{
"name": "bantu/ini-get-wrapper",
@ -40,6 +40,52 @@
},
"time": "2014-09-15T13:12:35+00:00"
},
{
"name": "chita/topological_sort",
"version": "v1.0.2",
"source": {
"type": "git",
"url": "https://github.com/CHItA/TopologicalSort.git",
"reference": "36918dd6b1f48d9deeccf84289184c9e5e91ef4b"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/CHItA/TopologicalSort/zipball/36918dd6b1f48d9deeccf84289184c9e5e91ef4b",
"reference": "36918dd6b1f48d9deeccf84289184c9e5e91ef4b",
"shasum": ""
},
"require": {
"php": ">=7.1.0"
},
"require-dev": {
"phpunit/phpunit": "^7.0"
},
"type": "library",
"autoload": {
"psr-4": {
"CHItA\\TopologicalSort\\": "src/"
},
"files": [
"src/TopologicalSort.php"
]
},
"notification-url": "https://packagist.org/downloads/",
"license": [
"MIT"
],
"authors": [
{
"name": "Máté Bartus",
"email": "mate.bartus@gmail.com"
}
],
"description": "Topological sort function",
"support": {
"issues": "https://github.com/CHItA/TopologicalSort/issues",
"source": "https://github.com/CHItA/TopologicalSort/tree/v1.0.2"
},
"time": "2020-12-31T15:47:10+00:00"
},
{
"name": "composer/ca-bundle",
"version": "1.2.8",

View file

@ -13,18 +13,27 @@
namespace phpbb\db\migration;
use Closure;
use LogicException;
use phpbb\config\config;
use phpbb\db\driver\driver_interface;
use phpbb\db\migrator;
use phpbb\db\tools\tools_interface;
use UnexpectedValueException;
use function CHItA\TopologicalSort\topologicalSort;
/**
* The schema generator generates the schema based on the existing migrations
*/
class schema_generator
{
/** @var \phpbb\config\config */
/** @var config */
protected $config;
/** @var \phpbb\db\driver\driver_interface */
/** @var driver_interface */
protected $db;
/** @var \phpbb\db\tools\tools_interface */
/** @var tools_interface */
protected $db_tools;
/** @var array */
@ -45,21 +54,26 @@ class schema_generator
/** @var array */
protected $table_names;
/** @var array */
protected $dependencies = array();
/**
* Constructor
* @param array $class_names
* @param \phpbb\config\config $config
* @param \phpbb\db\driver\driver_interface $db
* @param \phpbb\db\tools\tools_interface $db_tools
* @param config $config
* @param driver_interface $db
* @param tools_interface $db_tools
* @param string $phpbb_root_path
* @param string $php_ext
* @param string $table_prefix
* @param array $tables
*/
public function __construct(array $class_names, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, \phpbb\db\tools\tools_interface $db_tools, $phpbb_root_path, $php_ext, $table_prefix, $tables)
public function __construct(
array $class_names,
config $config,
driver_interface $db,
tools_interface $db_tools,
string $phpbb_root_path,
string $php_ext,
string $table_prefix,
array $tables)
{
$this->config = $config;
$this->db = $db;
@ -74,7 +88,11 @@ class schema_generator
/**
* Loads all migrations and their application state from the database.
*
* @return array
* @return array An array describing the database schema.
*
* @throws UnexpectedValueException If a migration tries to use an undefined schema change.
* @throws UnexpectedValueException If a dependency can't be resolved or there are circular
* dependencies between migrations.
*/
public function get_schema()
{
@ -84,148 +102,27 @@ class schema_generator
}
$migrations = $this->class_names;
$filter = function($class_name) {
return !migrator::is_migration($class_name);
};
$tree = array();
$check_dependencies = true;
while (!empty($migrations))
$edges = function($class_name) {
return $class_name::depends_on();
};
$apply_for_each = function($class_name) {
$this->apply_migration_to_schema($class_name);
};
try
{
foreach ($migrations as $key => $migration_class)
{
// Unset classes that are not a valid migration
if (\phpbb\db\migrator::is_migration($migration_class) === false)
{
unset($migrations[$key]);
continue;
}
$open_dependencies = array_diff($migration_class::depends_on(), $tree);
if (empty($open_dependencies))
{
$migration = new $migration_class($this->config, $this->db, $this->db_tools, $this->phpbb_root_path, $this->php_ext, $this->table_prefix, $this->table_names);
$tree[] = $migration_class;
$migration_key = array_search($migration_class, $migrations);
foreach ($migration->update_schema() as $change_type => $data)
{
if ($change_type === 'add_tables')
{
foreach ($data as $table => $table_data)
{
$this->tables[$table] = $table_data;
}
}
else if ($change_type === 'drop_tables')
{
foreach ($data as $table)
{
unset($this->tables[$table]);
}
}
else if ($change_type === 'add_columns')
{
foreach ($data as $table => $add_columns)
{
foreach ($add_columns as $column => $column_data)
{
if (isset($column_data['after']))
{
$columns = $this->tables[$table]['COLUMNS'];
$offset = array_search($column_data['after'], array_keys($columns));
unset($column_data['after']);
if ($offset === false)
{
$this->tables[$table]['COLUMNS'][$column] = array_values($column_data);
}
else
{
$this->tables[$table]['COLUMNS'] = array_merge(array_slice($columns, 0, $offset + 1, true), array($column => array_values($column_data)), array_slice($columns, $offset));
}
}
else
{
$this->tables[$table]['COLUMNS'][$column] = $column_data;
}
}
}
}
else if ($change_type === 'change_columns')
{
foreach ($data as $table => $change_columns)
{
foreach ($change_columns as $column => $column_data)
{
$this->tables[$table]['COLUMNS'][$column] = $column_data;
}
}
}
else if ($change_type === 'drop_columns')
{
foreach ($data as $table => $drop_columns)
{
if (is_array($drop_columns))
{
foreach ($drop_columns as $column)
{
unset($this->tables[$table]['COLUMNS'][$column]);
}
}
else
{
unset($this->tables[$table]['COLUMNS'][$drop_columns]);
}
}
}
else if ($change_type === 'add_unique_index')
{
foreach ($data as $table => $add_index)
{
foreach ($add_index as $key => $index_data)
{
$this->tables[$table]['KEYS'][$key] = array('UNIQUE', $index_data);
}
}
}
else if ($change_type === 'add_index')
{
foreach ($data as $table => $add_index)
{
foreach ($add_index as $key => $index_data)
{
$this->tables[$table]['KEYS'][$key] = array('INDEX', $index_data);
}
}
}
else if ($change_type === 'drop_keys')
{
foreach ($data as $table => $drop_keys)
{
foreach ($drop_keys as $key)
{
unset($this->tables[$table]['KEYS'][$key]);
}
}
}
else
{
var_dump($change_type);
}
}
unset($migrations[$migration_key]);
}
else if ($check_dependencies)
{
$this->dependencies = array_merge($this->dependencies, $open_dependencies);
}
}
// Only run this check after the first run
if ($check_dependencies)
{
$this->check_dependencies();
$check_dependencies = false;
}
topologicalSort($migrations, $edges, true, $apply_for_each, $filter);
}
catch (LogicException $e)
{
throw new UnexpectedValueException(
"Migrations either have circular dependencies or unsatisfiable dependencies."
);
}
ksort($this->tables);
@ -233,22 +130,216 @@ class schema_generator
}
/**
* Check if one of the migrations files' dependencies can't be resolved
* by the supplied list of migrations
*
* @throws \UnexpectedValueException If a dependency can't be resolved
*/
protected function check_dependencies()
* Apply the changes defined in the migration to the database schema.
*
* @param string $migration_class The name of the migration class.
*
* @throws UnexpectedValueException If a migration tries to use an undefined schema change.
*/
private function apply_migration_to_schema(string $migration_class)
{
// Strip duplicate values from array
$this->dependencies = array_unique($this->dependencies);
$migration = new $migration_class(
$this->config,
$this->db,
$this->db_tools,
$this->phpbb_root_path,
$this->php_ext,
$this->table_prefix,
$this->table_names
);
foreach ($this->dependencies as $dependency)
$column_map = [
'add_tables' => null,
'drop_tables' => null,
'add_columns' => 'COLUMNS',
'drop_columns' => 'COLUMNS',
'change_columns' => 'COLUMNS',
'add_index' => 'KEYS',
'add_unique_index' => 'KEYS',
'drop_keys' => 'KEYS',
];
$schema_changes = $migration->update_schema();
foreach ($schema_changes as $change_type => $changes)
{
if (!in_array($dependency, $this->class_names))
if (!array_key_exists($change_type, $column_map))
{
throw new \UnexpectedValueException("Unable to resolve the dependency '$dependency'");
throw new UnexpectedValueException("$migration_class contains undefined schema changes: $change_type.");
}
$split_position = strpos($change_type, '_');
$schema_change_type = substr($change_type, 0, $split_position);
$schema_type = substr($change_type, $split_position + 1);
$action = null;
switch ($schema_change_type)
{
case 'add':
case 'change':
$action = function(&$value, $changes, $value_transform = null) {
self::set_all($value, $changes, $value_transform);
};
break;
case 'drop':
$action = function(&$value, $changes, $value_transform = null) {
self::unset_all($value, $changes);
};
break;
default:
throw new UnexpectedValueException("$migration_class contains undefined schema changes: $change_type.");
}
switch ($schema_type)
{
case 'tables':
$action($this->tables, $changes);
break;
default:
$this->for_each_table(
$changes,
$action,
$column_map[$change_type],
self::get_value_transform($schema_change_type, $schema_type)
);
}
}
}
/**
* Apply `$callback` to each table specified in `$data`.
*
* @param array $data Array describing the schema changes.
* @param callable $callback Callback function to be applied.
* @param string|null $column Column of the `$this->tables` array for the table on which
* the change will be made or null.
* @param callable|null $value_transform Value transformation callback function or null.
*/
private function for_each_table(array $data, callable $callback, $column = null, $value_transform = null)
{
foreach ($data as $table => $values)
{
$target = &$this->tables[$table];
if ($column !== null)
{
$target = &$target[$column];
}
$callback($target, $values, $value_transform);
}
}
/**
* Set an array of key-value pairs in the schema.
*
* @param mixed $schema Reference to the schema entry.
* @param mixed $data Array of values to be set.
* @param callable|null $value_transform Callback to transform the value being set.
*/
private static function set_all(&$schema, $data, ?callable $value_transform = null)
{
$data = (!is_array($data)) ? [$data] : $data;
foreach ($data as $key => $change)
{
if (is_callable($value_transform))
{
$value_transform($schema, $key, $change);
}
else
{
$schema[$key] = $change;
}
}
}
/**
* Remove an array of values from the schema
*
* @param mixed $schema Reference to the schema entry.
* @param mixed $data Array of values to be removed.
*/
private static function unset_all(&$schema, $data)
{
$data = (!is_array($data)) ? [$data] : $data;
foreach ($data as $key)
{
unset($schema[$key]);
}
}
/**
* Logic for adding a new column to a table.
*
* @param array $value The table column entry.
* @param string $key The column name to add.
* @param array $change The column data.
*/
private static function handle_add_column(array &$value, string $key, array $change)
{
if (!array_key_exists('after', $change))
{
$value[$key] = $change;
return;
}
$after = $change['after'];
unset($change['after']);
if ($after === null)
{
$value[$key] = array_values($change);
return;
}
$offset = array_search($after, array_keys($value));
if ($offset === false)
{
$value[$key] = array_values($change);
return;
}
$value = array_merge(
array_slice($value, 0, $offset + 1, true),
[$key => array_values($change)],
array_slice($value, $offset)
);
}
/**
* Returns the value transform for the change.
*
* @param string $change_type The type of the change.
* @param string $schema_type The schema type on which the change is to be performed.
*
* @return Closure|null The value transformation callback or null if it is not needed.
*/
private static function get_value_transform(string $change_type, string $schema_type)
{
if ($change_type !== 'add')
{
return null;
}
switch ($schema_type)
{
case 'index':
return function(&$value, $key, $change) {
$value[$key] = ['INDEX', $change];
};
case 'unique_index':
return function(&$value, $key, $change) {
$value[$key] = ['UNIQUE', $change];
};
case 'columns':
return function(&$value, $key, $change) {
self::handle_add_column($value, $key, $change);
};
}
return null;
}
}

View file

@ -13,6 +13,13 @@
class phpbb_dbal_migration_dummy_order_0 extends \phpbb\db\migration\migration
{
public static function depends_on()
{
return [
'phpbb_dbal_migration_dummy_order',
];
}
function update_schema()
{
return array(

View file

@ -13,6 +13,13 @@
class phpbb_dbal_migration_dummy_order_1 extends \phpbb\db\migration\migration
{
public static function depends_on()
{
return [
'phpbb_dbal_migration_dummy_order',
];
}
function update_schema()
{
return array(

View file

@ -13,6 +13,13 @@
class phpbb_dbal_migration_dummy_order_2 extends \phpbb\db\migration\migration
{
public static function depends_on()
{
return [
'phpbb_dbal_migration_dummy_order',
];
}
function update_schema()
{
return array(

View file

@ -13,6 +13,13 @@
class phpbb_dbal_migration_dummy_order_3 extends \phpbb\db\migration\migration
{
public static function depends_on()
{
return [
'phpbb_dbal_migration_dummy_order',
];
}
function update_schema()
{
return array(

View file

@ -13,6 +13,13 @@
class phpbb_dbal_migration_dummy_order_4 extends \phpbb\db\migration\migration
{
public static function depends_on()
{
return [
'phpbb_dbal_migration_dummy_order',
];
}
function update_schema()
{
return array(

View file

@ -13,6 +13,13 @@
class phpbb_dbal_migration_dummy_order_5 extends \phpbb\db\migration\migration
{
public static function depends_on()
{
return [
'phpbb_dbal_migration_dummy_order',
];
}
function update_schema()
{
return array(