From cacaffee6e013e43b75212d49960922f88f9f69a Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Sat, 9 Feb 2013 20:56:42 -0600 Subject: [PATCH 1/5] [feature/migrations] Add language strings for migrations errors Unfulfillable returns string of the missing dependency name now if the migration is unfulfillable (this is significantly more helpful). PHPBB3-11351 --- phpBB/includes/db/migration/tool/config.php | 2 +- phpBB/includes/db/migration/tool/module.php | 6 +- .../includes/db/migration/tool/permission.php | 6 +- phpBB/includes/db/migrator.php | 16 +++--- phpBB/language/en/migrator.php | 55 +++++++++++++++++++ 5 files changed, 71 insertions(+), 14 deletions(-) create mode 100644 phpBB/language/en/migrator.php diff --git a/phpBB/includes/db/migration/tool/config.php b/phpBB/includes/db/migration/tool/config.php index d9cc20053e..458a25fb66 100644 --- a/phpBB/includes/db/migration/tool/config.php +++ b/phpBB/includes/db/migration/tool/config.php @@ -49,7 +49,7 @@ class phpbb_db_migration_tool_config implements phpbb_db_migration_tool_interfac { if (isset($this->config[$config_name])) { - throw new phpbb_db_migration_exception('CONFIG_ALREADY_EXISTS', $config_name); + throw new phpbb_db_migration_exception('CONFIG_ALREADY_EXIST', $config_name); } $this->config->set($config_name, $config_value, !$is_dynamic); diff --git a/phpBB/includes/db/migration/tool/module.php b/phpBB/includes/db/migration/tool/module.php index afe1f21ec5..4d7fae2bb0 100644 --- a/phpBB/includes/db/migration/tool/module.php +++ b/phpBB/includes/db/migration/tool/module.php @@ -242,14 +242,14 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac if (!$module_id) { - throw new phpbb_db_migration_exception('MODULE_PARENT_NOT_EXIST', $parent); + throw new phpbb_db_migration_exception('MODULE_NOT_EXIST', $parent); } $parent = $data['parent_id'] = $module_id; } else if (!$this->exists($class, false, $parent)) { - throw new phpbb_db_migration_exception('MODULE_PARENT_NOT_EXIST', $parent); + throw new phpbb_db_migration_exception('MODULE_NOT_EXIST', $parent); } if ($this->exists($class, $parent, $data['module_langname'])) @@ -477,7 +477,7 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac $result = $acp_modules->delete_module($module_id); if (!empty($result)) { - throw new phpbb_db_migration_exception('CANNOT_REMOVE_MODULE', $module_id); + throw new phpbb_db_migration_exception('MODULE_NOT_REMOVABLE', $module_id, $result); } } diff --git a/phpBB/includes/db/migration/tool/permission.php b/phpBB/includes/db/migration/tool/permission.php index 001d090f5a..4231fbe1dd 100644 --- a/phpBB/includes/db/migration/tool/permission.php +++ b/phpBB/includes/db/migration/tool/permission.php @@ -107,7 +107,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte { if ($this->exists($auth_option, $global)) { - throw new phpbb_db_migration_exception('PERMISSION_ALREADY_EXISTS', $auth_option); + throw new phpbb_db_migration_exception('PERMISSION_ALREADY_EXIST', $auth_option); } // We've added permissions, so set to true to notify the user. @@ -252,7 +252,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte if ($role_id) { - throw new phpbb_db_migration_exception('ROLE_ALREADY_EXISTS', $old_role_name); + return; } $sql = 'SELECT MAX(role_order) AS max_role_order @@ -290,7 +290,7 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte if (!$role_id) { - throw new phpbb_db_migration_exception('ROLE_NOT_EXISTS', $old_role_name); + throw new phpbb_db_migration_exception('ROLE_NOT_EXIST', $old_role_name); } $sql = 'UPDATE ' . ACL_ROLES_TABLE . " diff --git a/phpBB/includes/db/migrator.php b/phpBB/includes/db/migrator.php index 4456600b0a..3c311e96d7 100644 --- a/phpBB/includes/db/migrator.php +++ b/phpBB/includes/db/migrator.php @@ -228,9 +228,10 @@ class phpbb_db_migrator { foreach ($this->migrations as $name) { - if ($this->unfulfillable($name)) + $unfulfillable = $this->unfulfillable($name); + if ($unfulfillable !== false) { - throw new phpbb_db_migration_exception('MIGRATION NOT FULFILLABLE', $name); + throw new phpbb_db_migration_exception('MIGRATION_NOT_FULFILLABLE', $name, $unfulfillable); } } } @@ -674,13 +675,13 @@ 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 + * @return bool|string False if fulfillable, string of missing migration name if unfulfillable */ public function unfulfillable($name) { if (isset($this->migration_state[$name])) { - return false; + return $name; } if (!class_exists($name)) @@ -693,9 +694,10 @@ class phpbb_db_migrator foreach ($depends as $depend) { - if ($this->unfulfillable($depend)) + $unfulfillable = $this->unfulfillable($depend); + if ($unfulfillable !== false) { - return true; + return $unfulfillable; } } @@ -715,7 +717,7 @@ class phpbb_db_migrator { // skip unfulfillable migrations, but fulfillables mean we // are not finished yet - if ($this->unfulfillable($name)) + if ($this->unfulfillable($name) !== false) { continue; } diff --git a/phpBB/language/en/migrator.php b/phpBB/language/en/migrator.php new file mode 100644 index 0000000000..bc9fbc57af --- /dev/null +++ b/phpBB/language/en/migrator.php @@ -0,0 +1,55 @@ + 'The config setting "%s" unexpectedly already exists.', + 'CONFIG_NOT_EXIST' => 'The config setting "%s" unexpectedly does not exist.', + + 'GROUP_NOT_EXIST' => 'The group "%s" unexpectedly does not exist.', + + 'MIGRATION_NOT_FULFILLABLE' => 'The migration "%1$s" is not fulfillable, missing migration "%2$s".', + + 'MODULE_ALREADY_EXIST' => 'The module "%s" unexpectedly already exists.', + 'MODULE_ERROR' => 'An error occured while creating a module: %s', + 'MODULE_INFO_FILE_NOT_EXIST' => 'A required module info file is missing: %2$s', + 'MODULE_NOT_EXIST' => 'A required module does not exist: %s', + 'MODULE_NOT_REMOVABLE' => 'Module %1$s was unable to be removed: %2$s', + + 'PERMISSION_ALREADY_EXIST' => 'The permission setting "%s" unexpectedly already exists.', + 'PERMISSION_NOT_EXIST' => 'The permission setting "%s" unexpectedly does not exist.', + + 'ROLE_NOT_EXIST' => 'The permission role "%s" unexpectedly does not exist.', +)); From f409697137b0b1d01b663bb81d12f6179c3922e3 Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Sat, 9 Feb 2013 21:06:08 -0600 Subject: [PATCH 2/5] [feature/migrations] Add explanatory language string for migration errors This should be used if an exception is thrown to inform the user of what occurred. PHPBB3-11351 --- phpBB/language/en/migrator.php | 1 + 1 file changed, 1 insertion(+) diff --git a/phpBB/language/en/migrator.php b/phpBB/language/en/migrator.php index bc9fbc57af..84074c391c 100644 --- a/phpBB/language/en/migrator.php +++ b/phpBB/language/en/migrator.php @@ -40,6 +40,7 @@ $lang = array_merge($lang, array( 'GROUP_NOT_EXIST' => 'The group "%s" unexpectedly does not exist.', + 'MIGRATION_EXCEPTION_ERROR' => 'Something went wrong during the request and an exception was thrown. The changes made before the error occurred were reversed to the best of our abilities, but you should check the board for errors.', 'MIGRATION_NOT_FULFILLABLE' => 'The migration "%1$s" is not fulfillable, missing migration "%2$s".', 'MODULE_ALREADY_EXIST' => 'The module "%s" unexpectedly already exists.', From f18b096df945648aa3ecc2b8b914869871b1823f Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Sat, 9 Feb 2013 21:10:56 -0600 Subject: [PATCH 3/5] [feature/migrations] getParameters function for migration exception PHPBB3-11351 --- phpBB/includes/db/migration/exception.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/phpBB/includes/db/migration/exception.php b/phpBB/includes/db/migration/exception.php index ffdcd97780..b3abcdb5e5 100644 --- a/phpBB/includes/db/migration/exception.php +++ b/phpBB/includes/db/migration/exception.php @@ -52,4 +52,14 @@ class phpbb_db_migration_exception extends \Exception { return $this->message . ': ' . var_export($this->parameters, true); } + + /** + * Get the parameters + * + * @return array + */ + public function getParameters() + { + return $this->parameters; + } } From f9a1b27a99e30a41db4facd415b39a690614d6c6 Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Sat, 9 Feb 2013 21:16:39 -0600 Subject: [PATCH 4/5] [feature/migrations] Fix unfulfillable function Returned unfulfillable name in the wrong place previously PHPBB3-11351 --- phpBB/includes/db/migrator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/db/migrator.php b/phpBB/includes/db/migrator.php index 3c311e96d7..41d996b1e3 100644 --- a/phpBB/includes/db/migrator.php +++ b/phpBB/includes/db/migrator.php @@ -681,12 +681,12 @@ class phpbb_db_migrator { if (isset($this->migration_state[$name])) { - return $name; + return false; } if (!class_exists($name)) { - return true; + return $name; } $migration = $this->get_migration($name); From f6bb14569b0a777d3511b732201af9d1a415f2ed Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Fri, 15 Feb 2013 22:19:24 -0600 Subject: [PATCH 5/5] [feature/migrations] getLocalisedMessage function for migration exception PHPBB3-11351 --- phpBB/includes/db/migration/exception.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/phpBB/includes/db/migration/exception.php b/phpBB/includes/db/migration/exception.php index b3abcdb5e5..e84330dd71 100644 --- a/phpBB/includes/db/migration/exception.php +++ b/phpBB/includes/db/migration/exception.php @@ -62,4 +62,18 @@ class phpbb_db_migration_exception extends \Exception { return $this->parameters; } + + /** + * Get localised message (with $user->lang()) + * + * @param phpbb_user $user + * @return string + */ + public function getLocalisedMessage(phpbb_user $user) + { + $parameters = $this->getParameters(); + array_unshift($parameters, $this->getMessage()); + + return call_user_func_array(array($user, 'lang'), $parameters); + } }