diff --git a/phpBB/composer.json b/phpBB/composer.json index 3f9b428d1e..32b4725f37 100644 --- a/phpBB/composer.json +++ b/phpBB/composer.json @@ -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", diff --git a/phpBB/composer.lock b/phpBB/composer.lock index b5a011d4d4..8ba57523be 100644 --- a/phpBB/composer.lock +++ b/phpBB/composer.lock @@ -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", diff --git a/phpBB/phpbb/db/migration/schema_generator.php b/phpBB/phpbb/db/migration/schema_generator.php index 0d1f6e33b1..66b05623d4 100644 --- a/phpBB/phpbb/db/migration/schema_generator.php +++ b/phpBB/phpbb/db/migration/schema_generator.php @@ -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; + } } diff --git a/tests/dbal/migration/dummy_order_0.php b/tests/dbal/migration/dummy_order_0.php index 4caea76704..d0d12ed696 100644 --- a/tests/dbal/migration/dummy_order_0.php +++ b/tests/dbal/migration/dummy_order_0.php @@ -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( diff --git a/tests/dbal/migration/dummy_order_1.php b/tests/dbal/migration/dummy_order_1.php index ea512765fa..ff73291c23 100644 --- a/tests/dbal/migration/dummy_order_1.php +++ b/tests/dbal/migration/dummy_order_1.php @@ -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( diff --git a/tests/dbal/migration/dummy_order_2.php b/tests/dbal/migration/dummy_order_2.php index a7669b1ac8..58d195873c 100644 --- a/tests/dbal/migration/dummy_order_2.php +++ b/tests/dbal/migration/dummy_order_2.php @@ -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( diff --git a/tests/dbal/migration/dummy_order_3.php b/tests/dbal/migration/dummy_order_3.php index 501db09b7e..8c57f7580e 100644 --- a/tests/dbal/migration/dummy_order_3.php +++ b/tests/dbal/migration/dummy_order_3.php @@ -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( diff --git a/tests/dbal/migration/dummy_order_4.php b/tests/dbal/migration/dummy_order_4.php index 77bb5a24b3..d48a735bd3 100644 --- a/tests/dbal/migration/dummy_order_4.php +++ b/tests/dbal/migration/dummy_order_4.php @@ -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( diff --git a/tests/dbal/migration/dummy_order_5.php b/tests/dbal/migration/dummy_order_5.php index 8f92392a8b..8e13f69d5e 100644 --- a/tests/dbal/migration/dummy_order_5.php +++ b/tests/dbal/migration/dummy_order_5.php @@ -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(