diff options
author | Nathan Guse <nathaniel.guse@gmail.com> | 2013-01-09 14:27:01 -0600 |
---|---|---|
committer | Nathan Guse <nathaniel.guse@gmail.com> | 2013-01-09 16:44:08 -0600 |
commit | e3737978f76a962385a26de910959607d0ae0d30 (patch) | |
tree | b97e686e13305f42d3aa22374321fbad99c1edd2 | |
parent | e9bcea5d82fd086ebcf7634ab386623f34ea8d03 (diff) | |
download | forums-e3737978f76a962385a26de910959607d0ae0d30.tar forums-e3737978f76a962385a26de910959607d0ae0d30.tar.gz forums-e3737978f76a962385a26de910959607d0ae0d30.tar.bz2 forums-e3737978f76a962385a26de910959607d0ae0d30.tar.xz forums-e3737978f76a962385a26de910959607d0ae0d30.zip |
[feature/migrations] Fixing returns of callables and handling data state
Lots of comments and some other miscellaneous fixes.
PHPBB3-9737
-rw-r--r-- | phpBB/includes/db/db_tools.php | 1 | ||||
-rw-r--r-- | phpBB/includes/db/driver/mysqli.php | 1 | ||||
-rw-r--r-- | phpBB/includes/db/migration/exception.php | 10 | ||||
-rw-r--r-- | phpBB/includes/db/migration/migration.php | 35 | ||||
-rw-r--r-- | phpBB/includes/db/migration/tool/config.php | 40 | ||||
-rw-r--r-- | phpBB/includes/db/migration/tool/interface.php | 5 | ||||
-rw-r--r-- | phpBB/includes/db/migration/tool/module.php | 177 | ||||
-rw-r--r-- | phpBB/includes/db/migration/tool/permission.php | 106 | ||||
-rw-r--r-- | phpBB/includes/db/migrator.php | 158 |
9 files changed, 329 insertions, 204 deletions
diff --git a/phpBB/includes/db/db_tools.php b/phpBB/includes/db/db_tools.php index 5329d871c8..1d6823d37a 100644 --- a/phpBB/includes/db/db_tools.php +++ b/phpBB/includes/db/db_tools.php @@ -349,6 +349,7 @@ class phpbb_db_tools * Setter for {@link $return_statements return_statements}. * * @param bool $return_statements True if SQL should not be executed but returned as strings + * @return null */ public function set_return_statements($return_statements) { diff --git a/phpBB/includes/db/driver/mysqli.php b/phpBB/includes/db/driver/mysqli.php index 94921a22b7..7448bf1670 100644 --- a/phpBB/includes/db/driver/mysqli.php +++ b/phpBB/includes/db/driver/mysqli.php @@ -24,7 +24,6 @@ if (!defined('IN_PHPBB')) class phpbb_db_driver_mysqli extends phpbb_db_driver { var $multi_insert = true; - var $connect_error = ''; /** diff --git a/phpBB/includes/db/migration/exception.php b/phpBB/includes/db/migration/exception.php index 19fb39ab23..bd46a4e064 100644 --- a/phpBB/includes/db/migration/exception.php +++ b/phpBB/includes/db/migration/exception.php @@ -22,8 +22,15 @@ if (!defined('IN_PHPBB')) */ class phpbb_db_migration_exception extends \Exception { + /** @var array Extra parameters sent to exception to aid in debugging */ protected $parameters; + /** + * Throw an exception. + * + * First argument is the error message. + * Additional arguments will be output with the error message. + */ public function __construct() { $parameters = func_get_args(); @@ -33,6 +40,9 @@ class phpbb_db_migration_exception extends \Exception $this->parameters = $parameters; } + /** + * Output the error as a string + */ public function __toString() { return $this->message . ': ' . var_export($this->parameters, true); diff --git a/phpBB/includes/db/migration/migration.php b/phpBB/includes/db/migration/migration.php index c2c6da855b..5f1f0443db 100644 --- a/phpBB/includes/db/migration/migration.php +++ b/phpBB/includes/db/migration/migration.php @@ -26,22 +26,41 @@ if (!defined('IN_PHPBB')) */ abstract class phpbb_db_migration { + /** @var phpbb_config */ protected $config; + + /** @var phpbb_db_driver */ protected $db; + + /** @var phpbb_db_tools */ protected $db_tools; + + /** @var string */ protected $table_prefix; + /** @var string */ protected $phpbb_root_path; + + /** @var string */ protected $php_ext; + /** @var array Errors, if any occured */ protected $errors; - private $queries = array(); + /** @var array List of queries executed through $this->sql_query() */ + protected $queries = array(); /** - * Migration constructor + * Constructor + * + * @param phpbb_config $config + * @param phpbb_db_driver $db + * @param phpbb_db_tools $db_tools + * @param string $phpbb_root_path + * @param string $php_ext + * @param string $table_prefix */ - public function __construct($config, phpbb_db_driver $db, $db_tools, $phpbb_root_path, $php_ext, $table_prefix) + public function __construct(phpbb_config $config, phpbb_db_driver $db, phpbb_db_tools $db_tools, $phpbb_root_path, $php_ext, $table_prefix) { $this->config = $config; $this->db = $db; @@ -55,7 +74,7 @@ abstract class phpbb_db_migration } /** - * Defines other migrationsto be applied first (abstract method) + * Defines other migrations to be applied first (abstract method) * * @return array An array of migration class names */ @@ -67,7 +86,7 @@ abstract class phpbb_db_migration /** * Updates the database schema by providing a set of change instructions * - * @return array + * @return array Array of schema changes (compatible with db_tools->perform_schema_changes()) */ public function update_schema() { @@ -77,14 +96,18 @@ abstract class phpbb_db_migration /** * Updates data by returning a list of instructions to be executed * - * @return array + * @return array Array of data update instructions */ public function update_data() { + return array(); } /** * Wrapper for running queries to generate user feedback on updates + * + * @param string $sql SQL query to run on the database + * @return mixed Query result from db->sql_query() */ protected function sql_query($sql) { diff --git a/phpBB/includes/db/migration/tool/config.php b/phpBB/includes/db/migration/tool/config.php index 35fa3ce566..e7239436d2 100644 --- a/phpBB/includes/db/migration/tool/config.php +++ b/phpBB/includes/db/migration/tool/config.php @@ -7,11 +7,21 @@ * */ +/** +* Migration config tool +* +* @package db +*/ class phpbb_db_migration_tool_config implements phpbb_db_migration_tool_interface { /** @var phpbb_config */ - protected $config = null; + protected $config; + /** + * Constructor + * + * @param phpbb_config $config + */ public function __construct(phpbb_config $config) { $this->config = $config; @@ -26,13 +36,12 @@ class phpbb_db_migration_tool_config implements phpbb_db_migration_tool_interfac } /** - * Config Add - * - * This function allows you to add a config setting. + * Add a config setting. * * @param string $config_name The name of the config setting you would like to add * @param mixed $config_value The value of the config setting * @param bool $is_dynamic True if it is dynamic (changes very often) and should not be stored in the cache, false if not. + * @return null */ public function add($config_name, $config_value = '', $is_dynamic = false) { @@ -42,17 +51,14 @@ class phpbb_db_migration_tool_config implements phpbb_db_migration_tool_interfac } $this->config->set($config_name, $config_value, !$is_dynamic); - - return false; } /** - * Config Update - * - * This function allows you to update an existing config setting. + * Update an existing config setting. * * @param string $config_name The name of the config setting you would like to update * @param mixed $config_value The value of the config setting + * @return null */ public function update($config_name, $config_value = '') { @@ -62,18 +68,15 @@ class phpbb_db_migration_tool_config implements phpbb_db_migration_tool_interfac } $this->config->set($config_name, $config_value); - - return false; } /** - * Config Update If Equals - * - * This function allows you to update a config setting if the first argument equal to the current config value + * Update a config setting if the first argument equal to the current config value * * @param bool $compare If equal to the current config value, will be updated to the new config value, otherwise not * @param string $config_name The name of the config setting you would like to update * @param mixed $config_value The value of the config setting + * @return null */ public function update_if_equals($compare, $config_name, $config_value = '') { @@ -83,16 +86,13 @@ class phpbb_db_migration_tool_config implements phpbb_db_migration_tool_interfac } $this->config->set_atomic($config_name, $compare, $config_value); - - return false; } /** - * Config Remove - * - * This function allows you to remove an existing config setting. + * Remove an existing config setting. * * @param string $config_name The name of the config setting you would like to remove + * @return null */ public function remove($config_name) { @@ -102,7 +102,5 @@ class phpbb_db_migration_tool_config implements phpbb_db_migration_tool_interfac } $this->config->delete($config_name); - - return false; } } diff --git a/phpBB/includes/db/migration/tool/interface.php b/phpBB/includes/db/migration/tool/interface.php index 1815f5e8a2..5d10246ba1 100644 --- a/phpBB/includes/db/migration/tool/interface.php +++ b/phpBB/includes/db/migration/tool/interface.php @@ -7,6 +7,11 @@ * */ +/** +* Migration tool interface +* +* @package db +*/ interface phpbb_db_migration_tool_interface { /** diff --git a/phpBB/includes/db/migration/tool/module.php b/phpBB/includes/db/migration/tool/module.php index a503f08c01..f1b527bf21 100644 --- a/phpBB/includes/db/migration/tool/module.php +++ b/phpBB/includes/db/migration/tool/module.php @@ -7,27 +7,42 @@ * */ +/** +* Migration module management tool +* +* @package db +*/ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interface { /** @var phpbb_cache_service */ - protected $cache = null; + protected $cache; /** @var dbal */ - protected $db = null; + protected $db; /** @var phpbb_user */ - protected $user = null; + protected $user; /** @var string */ - protected $phpbb_root_path = null; + protected $phpbb_root_path; /** @var string */ - protected $php_ext = null; + protected $php_ext; /** @var string */ - protected $modules_table = null; + protected $modules_table; - public function __construct(phpbb_db_driver $db, $cache, $user, $phpbb_root_path, $php_ext, $modules_table) + /** + * Constructor + * + * @param phpbb_db_driver $db + * @param mixed $cache + * @param phpbb_user $user + * @param string $phpbb_root_path + * @param string $php_ext + * @param string $modules_table + */ + public function __construct(phpbb_db_driver $db, phpbb_cache_service $cache, phpbb_user $user, $phpbb_root_path, $php_ext, $modules_table) { $this->db = $db; $this->cache = $cache; @@ -53,7 +68,6 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac * @param string $class The module class(acp|mcp|ucp) * @param int|string|bool $parent The parent module_id|module_langname (0 for no parent). Use false to ignore the parent check and check class wide. * @param int|string $module The module_id|module_langname you would like to check for to see if it exists - * * @return bool true/false if module exists */ public function exists($class, $parent, $module) @@ -64,31 +78,28 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac return true; } - $class = $this->db->sql_escape($class); - $module = $this->db->sql_escape($module); - $parent_sql = ''; if ($parent !== false) { // Allows '' to be sent as 0 - $parent = (!$parent) ? 0 : $parent; + $parent = $parent ?: 0; if (!is_numeric($parent)) { $sql = 'SELECT module_id FROM ' . $this->modules_table . " WHERE module_langname = '" . $this->db->sql_escape($parent) . "' - AND module_class = '$class'"; + AND module_class = '" . $this->db->sql_escape($class) . "'"; $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); + $module_id = $this->db->sql_fetchfield('module_id'); $this->db->sql_freeresult($result); - if (!$row) + if (!$module_id) { return false; } - $parent_sql = 'AND parent_id = ' . (int) $row['module_id']; + $parent_sql = 'AND parent_id = ' . (int) $module_id; } else { @@ -98,14 +109,14 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac $sql = 'SELECT module_id FROM ' . $this->modules_table . " - WHERE module_class = '$class' + WHERE module_class = '" . $this->db->sql_escape($class) . "' $parent_sql - AND " . ((is_numeric($module)) ? 'module_id = ' . (int) $module : "module_langname = '$module'"); + AND " . ((is_numeric($module)) ? 'module_id = ' . (int) $module : "module_langname = '" . $this->db->sql_escape($module) . "'"); $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); + $module_id = $this->db->sql_fetchfield('module_id'); $this->db->sql_freeresult($result); - if ($row) + if ($module_id) { return true; } @@ -123,29 +134,30 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac * @param array $data an array of the data on the new module. This can be setup in two different ways. * 1. The "manual" way. For inserting a category or one at a time. It will be merged with the base array shown a bit below, * but at the least requires 'module_langname' to be sent, and, if you want to create a module (instead of just a category) you must send module_basename and module_mode. - * array( - * 'module_enabled' => 1, - * 'module_display' => 1, - * 'module_basename' => '', - * 'module_class' => $class, - * 'parent_id' => (int) $parent, - * 'module_langname' => '', - * 'module_mode' => '', - * 'module_auth' => '', - * ) + * array( + * 'module_enabled' => 1, + * 'module_display' => 1, + * 'module_basename' => '', + * 'module_class' => $class, + * 'parent_id' => (int) $parent, + * 'module_langname' => '', + * 'module_mode' => '', + * 'module_auth' => '', + * ) * 2. The "automatic" way. For inserting multiple at a time based on the specs in the info file for the module(s). For this to work the modules must be correctly setup in the info file. * An example follows (this would insert the settings, log, and flag modes from the includes/acp/info/acp_asacp.php file): - * array( - * 'module_basename' => 'asacp', - * 'modes' => array('settings', 'log', 'flag'), - * ) + * array( + * 'module_basename' => 'asacp', + * 'modes' => array('settings', 'log', 'flag'), + * ) * Optionally you may not send 'modes' and it will insert all of the modules in that info file. - * @param string|bool $include_path If you would like to use a custom include path, specify that here + * @param string|bool $include_path If you would like to use a custom include path, specify that here + * @return null */ public function add($class, $parent = 0, $data = array(), $include_path = false) { - // Allows '' to be sent as 0 - $parent = (!$parent) ? 0 : $parent; + // Allows '' to be sent as 0 + $parent = $parent ?: 0; // allow sending the name as a string in $data to create a category if (!is_array($data)) @@ -155,18 +167,16 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac if (!isset($data['module_langname'])) { - /** - * @TODO does not work with 3.1 modules yet, but must continue for old 3.0 versions for - * upgrades from a 3.0.x version to 3.1 - */ // The "automatic" way $basename = (isset($data['module_basename'])) ? $data['module_basename'] : ''; $basename = str_replace(array('/', '\\'), '', $basename); $class = str_replace(array('/', '\\'), '', $class); + + $include_path = ($include_path === false) ? $this->phpbb_root_path . 'includes/' : $include_path; $info_file = "$class/info/$basename.{$this->php_ext}"; // The manual and automatic ways both failed... - if (!file_exists((($include_path === false) ? $this->phpbb_root_path . 'includes/' : $include_path) . $info_file)) + if (!file_exists($include_path . $info_file)) { throw new phpbb_db_migration_exception('MODULE_INFO_FILE_NOT_EXIST', $class, $info_file); } @@ -175,7 +185,7 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac if (!class_exists($classname)) { - include((($include_path === false) ? $this->phpbb_root_path . 'includes/' : $include_path) . $info_file); + include($include_path . $info_file); } $info = new $classname; @@ -206,26 +216,25 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac } // The "manual" way - add_log('admin', 'LOG_MODULE_ADD', ((isset($this->user->lang[$data['module_langname']])) ? $this->user->lang[$data['module_langname']] : $data['module_langname'])); - - $class = $this->db->sql_escape($class); + $module_log_name = ((isset($this->user->lang[$data['module_langname']])) ? $this->user->lang[$data['module_langname']] : $data['module_langname']); + add_log('admin', 'LOG_MODULE_ADD', $module_log_name); if (!is_numeric($parent)) { $sql = 'SELECT module_id FROM ' . $this->modules_table . " WHERE module_langname = '" . $this->db->sql_escape($parent) . "' - AND module_class = '$class'"; + AND module_class = '" . $this->db->sql_escape($class) . "'"; $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); + $module_id = $this->db->sql_fetchfield('module_id'); $this->db->sql_freeresult($result); - if (!$row) + if (!$module_id) { throw new phpbb_db_migration_exception('MODULE_PARENT_NOT_EXIST', $parent); } - $parent = $data['parent_id'] = $row['module_id']; + $parent = $data['parent_id'] = $module_id; } else if (!$this->exists($class, false, $parent)) { @@ -270,46 +279,46 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac if (isset($data['before']) && $data['before']) { $sql = 'SELECT left_id - FROM ' . $this->modules_table . ' - WHERE module_class = \'' . $class . '\' - AND parent_id = ' . (int) $parent . ' - AND module_langname = \'' . $this->db->sql_escape($data['before']) . '\''; + FROM ' . $this->modules_table . " + WHERE module_class = '" . $this->db->sql_escape($class) . "' + AND parent_id = " . (int) $parent . " + AND module_langname = '" . $this->db->sql_escape($data['before']) . "'"; $this->db->sql_query($sql); - $to_left = $this->db->sql_fetchfield('left_id'); + $to_left = (int) $this->db->sql_fetchfield('left_id'); $sql = 'UPDATE ' . $this->modules_table . " SET left_id = left_id + 2, right_id = right_id + 2 - WHERE module_class = '$class' + WHERE module_class = '" . $this->db->sql_escape($class) . "' AND left_id >= $to_left AND left_id < {$module_data['left_id']}"; $this->db->sql_query($sql); $sql = 'UPDATE ' . $this->modules_table . " SET left_id = $to_left, right_id = " . ($to_left + 1) . " - WHERE module_class = '$class' + WHERE module_class = '" . $this->db->sql_escape($class) . "' AND module_id = {$module_data['module_id']}"; $this->db->sql_query($sql); } else if (isset($data['after']) && $data['after']) { $sql = 'SELECT right_id - FROM ' . $this->modules_table . ' - WHERE module_class = \'' . $class . '\' - AND parent_id = ' . (int) $parent . ' - AND module_langname = \'' . $this->db->sql_escape($data['after']) . '\''; + FROM ' . $this->modules_table . " + WHERE module_class = '" . $this->db->sql_escape($class) . "' + AND parent_id = " . (int) $parent . " + AND module_langname = '" . $this->db->sql_escape($data['after']) . "'"; $this->db->sql_query($sql); - $to_right = $this->db->sql_fetchfield('right_id'); + $to_right = (int) $this->db->sql_fetchfield('right_id'); $sql = 'UPDATE ' . $this->modules_table . " SET left_id = left_id + 2, right_id = right_id + 2 - WHERE module_class = '$class' + WHERE module_class = '" . $this->db->sql_escape($class) . "' AND left_id >= $to_right AND left_id < {$module_data['left_id']}"; $this->db->sql_query($sql); $sql = 'UPDATE ' . $this->modules_table . ' SET left_id = ' . ($to_right + 1) . ', right_id = ' . ($to_right + 2) . " - WHERE module_class = '$class' + WHERE module_class = '" . $this->db->sql_escape($class) . "' AND module_id = {$module_data['module_id']}"; $this->db->sql_query($sql); } @@ -317,8 +326,6 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac // Clear the Modules Cache $this->cache->destroy("_modules_$class"); - - return false; } /** @@ -330,6 +337,7 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac * @param int|string|bool $parent The parent module_id|module_langname (0 for no parent). Use false to ignore the parent check and check class wide. * @param int|string $module The module id|module_langname * @param string|bool $include_path If you would like to use a custom include path, specify that here + * @return null */ public function remove($class, $parent = 0, $module = '', $include_path = false) { @@ -351,9 +359,11 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac // Automatic method $basename = str_replace(array('/', '\\'), '', $module['module_basename']); $class = str_replace(array('/', '\\'), '', $class); + + $include_path = ($include_path === false) ? $this->phpbb_root_path . 'includes/' : $include_path; $info_file = "$class/info/$basename.{$this->php_ext}"; - if (!file_exists((($include_path === false) ? $this->phpbb_root_path . 'includes/' : $include_path) . $info_file)) + if (!file_exists($include_path . $info_file)) { throw new phpbb_db_migration_exception('MODULE_NOT_EXIST', $info_file); } @@ -362,7 +372,7 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac if (!class_exists($classname)) { - include((($include_path === false) ? $this->phpbb_root_path . 'includes/' : $include_path) . $info_file); + include($include_path . $info_file); } $info = new $classname; @@ -376,12 +386,9 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac $this->remove($class, $parent, $info['title']) . '<br />'; } } - return false; } else { - $class = $this->db->sql_escape($class); - if (!$this->exists($class, $parent, $module)) { throw new phpbb_db_migration_exception('MODULE_NOT_EXIST', ((isset($this->user->lang[$module])) ? $this->user->lang[$module] : $module)); @@ -391,20 +398,20 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac if ($parent !== false) { // Allows '' to be sent as 0 - $parent = (!$parent) ? 0 : $parent; + $parent = ($parent) ?: 0; if (!is_numeric($parent)) { $sql = 'SELECT module_id FROM ' . $this->modules_table . " WHERE module_langname = '" . $this->db->sql_escape($parent) . "' - AND module_class = '$class'"; + AND module_class = '" . $this->db->sql_escape($class) . "'"; $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); + $module_id = $this->db->sql_fetchfield('module_id'); $this->db->sql_freeresult($result); // we know it exists from the module_exists check - $parent_sql = 'AND parent_id = ' . (int) $row['module_id']; + $parent_sql = 'AND parent_id = ' . (int) $module_id; } else { @@ -415,16 +422,15 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac $module_ids = array(); if (!is_numeric($module)) { - $module = $this->db->sql_escape($module); $sql = 'SELECT module_id FROM ' . $this->modules_table . " - WHERE module_langname = '$module' - AND module_class = '$class' - $parent_sql"; + WHERE module_langname = '" . $this->db->sql_escape($module) . "' + AND module_class = '" . $this->db->sql_escape($class) . "' + $parent_sql"; $result = $this->db->sql_query($sql); - while ($row = $this->db->sql_fetchrow($result)) + while ($module_id = $this->db->sql_fetchfield('module_id')) { - $module_ids[] = (int) $row['module_id']; + $module_ids[] = (int) $module_id; } $this->db->sql_freeresult($result); @@ -436,13 +442,12 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac $sql = 'SELECT module_langname FROM ' . $this->modules_table . " WHERE module_id = $module - AND module_class = '$class' - $parent_sql"; + AND module_class = '" . $this->db->sql_escape($class) . "' + $parent_sql"; $result = $this->db->sql_query($sql); - $row = $this->db->sql_fetchrow($result); + $module_name = $this->db->sql_fetchfield('module_id'); $this->db->sql_freeresult($result); - $module_name = $row['module_langname']; $module_ids[] = $module; } @@ -464,8 +469,6 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac } $this->cache->destroy("_modules_$class"); - - return false; } } } diff --git a/phpBB/includes/db/migration/tool/permission.php b/phpBB/includes/db/migration/tool/permission.php index ebe404bc62..97fdbc0df9 100644 --- a/phpBB/includes/db/migration/tool/permission.php +++ b/phpBB/includes/db/migration/tool/permission.php @@ -7,24 +7,38 @@ * */ +/** +* Migration permission management tool +* +* @package db +*/ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_interface { /** @var phpbb_auth */ - protected $auth = null; + protected $auth; /** @var phpbb_cache_service */ - protected $cache = null; + protected $cache; /** @var dbal */ - protected $db = null; + protected $db; /** @var string */ - protected $phpbb_root_path = null; + protected $phpbb_root_path; /** @var string */ - protected $php_ext = null; + protected $php_ext; - public function __construct(phpbb_db_driver $db, $cache, phpbb_auth $auth, $phpbb_root_path, $php_ext) + /** + * Constructor + * + * @param phpbb_db_driver $db + * @param mixed $cache + * @param phpbb_auth $auth + * @param string $phpbb_root_path + * @param string $php_ext + */ + public function __construct(phpbb_db_driver $db, phpbb_cache_service $cache, phpbb_auth $auth, $phpbb_root_path, $php_ext) { $this->db = $db; $this->cache = $cache; @@ -48,7 +62,6 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte * * @param string $auth_option The name of the permission (auth) option * @param bool $global True for checking a global permission setting, False for a local permission setting - * * @return bool true if it exists, false if not */ public function exists($auth_option, $global = true) @@ -86,8 +99,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte * * @param string $auth_option The name of the permission (auth) option * @param bool $global True for checking a global permission setting, False for a local permission setting - * - * @return result + * @return null */ public function add($auth_option, $global = true, $copy_from = false) { @@ -152,7 +164,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte } $this->db->sql_freeresult($result); - if (sizeof($sql_ary)) + if (!empty($sql_ary)) { $this->db->sql_multi_insert($table, $sql_ary); } @@ -160,8 +172,6 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte $auth_admin->acl_clear_prefetch(); } - - return false; } /** @@ -171,8 +181,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte * * @param string $auth_option The name of the permission (auth) option * @param bool $global True for checking a global permission setting, False for a local permission setting - * - * @return result + * @return null */ public function remove($auth_option, $global = true) { @@ -197,7 +206,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte $row = $this->db->sql_fetchrow($result); $this->db->sql_freeresult($result); - $id = $row['auth_option_id']; + $id = (int) $row['auth_option_id']; // If it is a local and global permission, do not remove the row! :P if ($row['is_global'] && $row['is_local']) @@ -210,17 +219,17 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte else { // Delete time - $this->db->sql_query('DELETE FROM ' . ACL_GROUPS_TABLE . ' WHERE auth_option_id = ' . $id); - $this->db->sql_query('DELETE FROM ' . ACL_ROLES_DATA_TABLE . ' WHERE auth_option_id = ' . $id); - $this->db->sql_query('DELETE FROM ' . ACL_USERS_TABLE . ' WHERE auth_option_id = ' . $id); - $this->db->sql_query('DELETE FROM ' . ACL_OPTIONS_TABLE . ' WHERE auth_option_id = ' . $id); + $tables = array(ACL_GROUPS_TABLE, ACL_ROLES_DATA_TABLE, ACL_USERS_TABLE, ACL_OPTIONS_TABLE); + foreach ($tables as $table) + { + $this->db->sql_query('DELETE FROM ' . $table . ' + WHERE auth_option_id = ' . $id); + } } // Purge the auth cache $this->cache->destroy('_acl_options'); $this->auth->acl_clear_prefetch(); - - return false; } /** @@ -228,6 +237,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte * * @param string $role_name The new role name * @param sting $role_type The type (u_, m_, a_) + * @return null */ public function role_add($role_name, $role_type = '', $role_description = '') { @@ -235,18 +245,18 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte FROM ' . ACL_ROLES_TABLE . " WHERE role_name = '" . $this->db->sql_escape($role_name) . "'"; $this->db->sql_query($sql); - $role_id = $this->db->sql_fetchfield('role_id'); + $role_id = (int) $this->db->sql_fetchfield('role_id'); if ($role_id) { throw new phpbb_db_migration_exception('ROLE_ALREADY_EXISTS', $old_role_name); } - $sql = 'SELECT MAX(role_order) AS max + $sql = 'SELECT MAX(role_order) AS max_role_order FROM ' . ACL_ROLES_TABLE . " WHERE role_type = '" . $this->db->sql_escape($role_type) . "'"; $this->db->sql_query($sql); - $role_order = $this->db->sql_fetchfield('max'); + $role_order = (int) $this->db->sql_fetchfield('max_role_order'); $role_order = (!$role_order) ? 1 : $role_order + 1; $sql_ary = array( @@ -258,8 +268,6 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte $sql = 'INSERT INTO ' . ACL_ROLES_TABLE . ' ' . $this->db->sql_build_array('INSERT', $sql_ary); $this->db->sql_query($sql); - - return false; } /** @@ -267,6 +275,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte * * @param string $old_role_name The old role name * @param string $new_role_name The new role name + * @return null */ public function role_update($old_role_name, $new_role_name = '') { @@ -274,7 +283,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte FROM ' . ACL_ROLES_TABLE . " WHERE role_name = '" . $this->db->sql_escape($old_role_name) . "'"; $this->db->sql_query($sql); - $role_id = $this->db->sql_fetchfield('role_id'); + $role_id = (int) $this->db->sql_fetchfield('role_id'); if (!$role_id) { @@ -285,14 +294,13 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte SET role_name = '" . $this->db->sql_escape($new_role_name) . "' WHERE role_name = '" . $this->db->sql_escape($old_role_name) . "'"; $this->db->sql_query($sql); - - return false; } /** * Remove a permission role * * @param string $role_name The role name to remove + * @return null */ public function role_remove($role_name) { @@ -300,7 +308,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte FROM ' . ACL_ROLES_TABLE . " WHERE role_name = '" . $this->db->sql_escape($role_name) . "'"; $this->db->sql_query($sql); - $role_id = $this->db->sql_fetchfield('role_id'); + $role_id = (int) $this->db->sql_fetchfield('role_id'); if (!$role_id) { @@ -316,8 +324,6 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte $this->db->sql_query($sql); $this->auth->acl_clear_prefetch(); - - return false; } /** @@ -329,6 +335,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte * @param string|array $auth_option The auth_option or array of auth_options you would like to set * @param string $type The type (role|group) * @param bool $has_permission True if you want to give them permission, false if you want to deny them permission + * @return null */ public function permission_set($name, $auth_option = array(), $type = 'role', $has_permission = true) { @@ -344,13 +351,13 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte $result = $this->db->sql_query($sql); while ($row = $this->db->sql_fetchrow($result)) { - $new_auth[] = $row['auth_option_id']; + $new_auth[] = (int) $row['auth_option_id']; } $this->db->sql_freeresult($result); - if (!sizeof($new_auth)) + if (empty($new_auth)) { - return false; + return; } $current_auth = array(); @@ -364,7 +371,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte FROM ' . ACL_ROLES_TABLE . " WHERE role_name = '" . $this->db->sql_escape($name) . "'"; $this->db->sql_query($sql); - $role_id = $this->db->sql_fetchfield('role_id'); + $role_id = (int) $this->db->sql_fetchfield('role_id'); if (!$role_id) { @@ -387,7 +394,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte FROM ' . GROUPS_TABLE . " WHERE group_name = '" . $this->db->sql_escape($name) . "'"; $this->db->sql_query($sql); - $group_id = $this->db->sql_fetchfield('group_id'); + $group_id = (int) $this->db->sql_fetchfield('group_id'); if (!$group_id) { @@ -401,7 +408,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte AND auth_role_id <> 0 AND forum_id = 0'; $this->db->sql_query($sql); - $role_id = $this->db->sql_fetchfield('auth_role_id'); + $role_id = (int) $this->db->sql_fetchfield('auth_role_id'); if ($role_id) { $sql = 'SELECT role_name @@ -437,7 +444,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte 'role_id' => $role_id, 'auth_option_id' => $auth_option_id, 'auth_setting' => $has_permission, - ); + ); } } @@ -453,7 +460,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte 'group_id' => $group_id, 'auth_option_id' => $auth_option_id, 'auth_setting' => $has_permission, - ); + ); } } @@ -462,8 +469,6 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte } $this->auth->acl_clear_prefetch(); - - return false; } /** @@ -474,6 +479,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte * @param string $name The name of the role/group * @param string|array $auth_option The auth_option or array of auth_options you would like to set * @param string $type The type (role|group) + * @return null */ public function permission_unset($name, $auth_option = array(), $type = 'role') { @@ -489,13 +495,13 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte $result = $this->db->sql_query($sql); while ($row = $this->db->sql_fetchrow($result)) { - $to_remove[] = $row['auth_option_id']; + $to_remove[] = (int) $row['auth_option_id']; } $this->db->sql_freeresult($result); - if (!sizeof($to_remove)) + if (empty($to_remove)) { - return false; + return; } $type = (string) $type; // Prevent PHP bug. @@ -507,11 +513,11 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte FROM ' . ACL_ROLES_TABLE . " WHERE role_name = '" . $this->db->sql_escape($name) . "'"; $this->db->sql_query($sql); - $role_id = $this->db->sql_fetchfield('role_id'); + $role_id = (int) $this->db->sql_fetchfield('role_id'); if (!$role_id) { - throw new phpbb_db_migration_exception('ROLE_NOT_EXIST', $name); + throw new phpbb_db_migration_exception('ROLE_NOT_EXIST', $name); } $sql = 'DELETE FROM ' . ACL_ROLES_DATA_TABLE . ' @@ -524,7 +530,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte FROM ' . GROUPS_TABLE . " WHERE group_name = '" . $this->db->sql_escape($name) . "'"; $this->db->sql_query($sql); - $group_id = $this->db->sql_fetchfield('group_id'); + $group_id = (int) $this->db->sql_fetchfield('group_id'); if (!$group_id) { @@ -537,7 +543,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte WHERE group_id = ' . $group_id . ' AND auth_role_id <> 0'; $this->db->sql_query($sql); - $role_id = $this->db->sql_fetchfield('auth_role_id'); + $role_id = (int) $this->db->sql_fetchfield('auth_role_id'); if ($role_id) { $sql = 'SELECT role_name @@ -556,7 +562,5 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte } $this->auth->acl_clear_prefetch(); - - return false; } } diff --git a/phpBB/includes/db/migrator.php b/phpBB/includes/db/migrator.php index 2fd795b659..2ba1eba427 100644 --- a/phpBB/includes/db/migrator.php +++ b/phpBB/includes/db/migrator.php @@ -22,26 +22,40 @@ if (!defined('IN_PHPBB')) */ class phpbb_db_migrator { + /** @var phpbb_config */ protected $config; + + /** @var phpbb_db_driver */ protected $db; + + /** @var phpbb_db_tools */ protected $db_tools; + + /** @var string */ protected $table_prefix; + /** @var string */ protected $phpbb_root_path; + + /** @var string */ protected $php_ext; + /** @var string */ protected $migrations_table; + + /** @var array State of all migrations (SELECT * FROM migrations table) */ protected $migration_state; + /** @var array Array of all migrations available to be run */ protected $migrations = array(); - /** @var string Name of the last migration run */ + /** @var array 'name' and 'class' of the last migration run */ public $last_run_migration = false; /** * Constructor of the database migrator */ - public function __construct($config, phpbb_db_driver $db, $db_tools, $migrations_table, $phpbb_root_path, $php_ext, $table_prefix, $tools) + public function __construct(phpbb_config $config, phpbb_db_driver $db, phpbb_db_tools $db_tools, $migrations_table, $phpbb_root_path, $php_ext, $table_prefix, $tools) { $this->config = $config; $this->db = $db; @@ -96,17 +110,42 @@ class phpbb_db_migrator /** * Load migration data files from a directory * - * @param string $path + * This does not loop through sub-directories. + * Migration data files loaded with this function MUST contain + * ONLY ONE class in them (or an exception will be thrown). + * + * @param string $path Path to migration data files + * @param bool $check_fulfillable If TRUE (default), we will check + * if all of the migrations are fulfillable after loading them. + * If FALSE, we will not check. You SHOULD check at least once + * to prevent errors (if including multiple directories, check + * with the last call to prevent throwing errors unnecessarily). * @return array Array of migrations with names */ - public function load_migrations($path) + public function load_migrations($path, $check_fulfillable = true) { $handle = opendir($path); while (($file = readdir($handle)) !== false) { if (strpos($file, '_') !== 0 && strrpos($file, '.' . $this->php_ext) === (strlen($file) - strlen($this->php_ext) - 1)) { - $name = 'phpbb_db_migration_data_' . substr($file, 0, -(strlen($this->php_ext) + 1)); + // We try to find what class existed by comparing the classes declared before and after including the file. + $declared_classes = get_declared_classes(); + + include ($path . $file); + + $added_classes = array_diff(get_declared_classes(), $declared_classes); + + if ( + // The phpbb_db_migrations class may not have been loaded until now, so make sure to ignore it. + !(sizeof($added_classes) == 2 && in_array('phpbb_db_migration', $added_classes)) && + sizeof($added_classes) != 1 + ) + { + throw new phpbb_db_migration_exception('MIGRATION DATA FILE INVALID', $path . $file); + } + + $name = array_pop($added_classes); if (!in_array($name, $this->migrations)) { @@ -115,11 +154,14 @@ class phpbb_db_migrator } } - foreach ($this->migrations as $name) + if ($check_fulfillable) { - if ($this->unfulfillable($name)) + foreach ($this->migrations as $name) { - throw new phpbb_db_migration_exception('MIGRATION NOT FULFILLABLE', $name); + if ($this->unfulfillable($name)) + { + throw new phpbb_db_migration_exception('MIGRATION NOT FULFILLABLE', $name); + } } } @@ -131,6 +173,8 @@ class phpbb_db_migrator * * The update step can either be a schema or a (partial) data update. To * check if update() needs to be called again use the finished() method. + * + * @return null */ public function update() { @@ -207,9 +251,11 @@ class phpbb_db_migrator } else { - $this->process_data_step($migration); - $state['migration_data_done'] = true; - $state['migration_end_time'] = time(); + $state = $this->process_data_step($migration); + + $state['migration_data_state'] = $state; + $state['migration_data_done'] = ($state === true); + $state['migration_end_time'] = ($state === true) ? time() : 0; } $sql = 'UPDATE ' . $this->migrations_table . ' @@ -222,41 +268,74 @@ class phpbb_db_migrator 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 + * + * @param phpbb_db_migration $migration + * @return mixed migration status or bool true if everything completed successfully + */ protected function process_data_step($migration) { - //$continue = false; $steps = $migration->update_data(); foreach ($steps as $step) { - $continue = $this->run_step($step); - - /*if ($continue === false) + try { - return false; - }*/ + // Result will be null or true if everything completed correctly + $result = $this->run_step($step); + if ($result !== null && $result !== true) + { + return $result; + } + } + catch (phpbb_db_migration_exception $e) + { + // We should try rolling back here + + echo $e; + die(); + } } - //return $continue; + return true; } + /** + * Run a single step + * + * An exception should be thrown if an error occurs + * + * @param mixed $step + * @return null + */ protected function run_step($step) { - try - { - $callable_and_parameters = $this->get_callable_from_step($step); - $callable = $callable_and_parameters[0]; - $parameters = $callable_and_parameters[1]; + $callable_and_parameters = $this->get_callable_from_step($step); + $callable = $callable_and_parameters[0]; + $parameters = $callable_and_parameters[1]; - return call_user_func_array($callable, $parameters); - } - catch (phpbb_db_migration_exception $e) - { - echo $e; - die(); - } + return call_user_func_array($callable, $parameters); } + /** + * Get a callable statement from a data step + * + * @param mixed $step Data step from migration + * @return array Array with parameters for call_user_func_array(), 0 is the callable, 1 is parameters + */ public function get_callable_from_step($step) { $type = $step[0]; @@ -291,6 +370,7 @@ class phpbb_db_migrator $callable_and_parameters = $this->get_callable_from_step($step); $callable = $callable_and_parameters[0]; $sub_parameters = $callable_and_parameters[1]; + return array( function ($condition) use ($callable, $sub_parameters) { return call_user_func_array($callable, $sub_parameters); @@ -325,12 +405,19 @@ class phpbb_db_migrator return array( array($this->tools[$class], $method), - $parameters + $parameters, ); break; } } + /** + * Insert migration row into the database + * + * @param string $name Name of the migration + * @param array $state + * @return null + */ protected function insert_migration($name, $state) { $migration_row = $state; @@ -346,8 +433,8 @@ class phpbb_db_migrator /** * Checks if a migration's dependencies can even theoretically be satisfied. * - * @param string $name The class name of the migration - * @return bool Whether the migration cannot be fulfilled + * @param string $name The class name of the migration + * @return bool Whether the migration cannot be fulfilled */ public function unfulfillable($name) { @@ -406,11 +493,6 @@ class phpbb_db_migrator return true; } - protected function apply_schema_changes($schema_changes) - { - $this->db_tools->perform_schema_changes($schema_changes); - } - /** * Helper to get a migration * |