From 1b81bf5b2370c045a6369705d2a11a2b35fe2281 Mon Sep 17 00:00:00 2001 From: CHItA Date: Fri, 5 Jun 2015 17:43:30 +0200 Subject: [PATCH] [ticket/13740] Add better progress handling, also add log messages PHPBB3-13740 --- phpBB/assets/javascript/installer.js | 9 +- .../services_install_obtain_data.yml | 2 + .../services_install_requirements.yml | 2 + .../invalid_service_name_exception.php | 69 +++++++++ .../exception/module_not_found_exception.php | 42 ++++++ .../exception/task_not_found_exception.php | 42 ++++++ phpBB/install/helper/config.php | 11 +- phpBB/install/installer.php | 134 ++++++++++++++---- phpBB/install/module/obtain_data/module.php | 38 ----- phpBB/install/module/requirements/module.php | 11 +- phpBB/install/module_base.php | 126 +++++++++++----- phpBB/language/en/install.php | 15 ++ 12 files changed, 393 insertions(+), 108 deletions(-) create mode 100644 phpBB/install/exception/invalid_service_name_exception.php create mode 100644 phpBB/install/exception/module_not_found_exception.php create mode 100644 phpBB/install/exception/task_not_found_exception.php diff --git a/phpBB/assets/javascript/installer.js b/phpBB/assets/javascript/installer.js index 8afac0da78..a9a315c0d5 100644 --- a/phpBB/assets/javascript/installer.js +++ b/phpBB/assets/javascript/installer.js @@ -204,13 +204,14 @@ * @param progressLimit */ function incrementFiller($progressText, $progressFiller, progressLimit) { + if (currentProgress >= progressLimit || currentProgress >= 100) { + clearInterval(progressTimer); + return; + } + currentProgress++; $progressText.text(currentProgress + '%'); $progressFiller.css('width', currentProgress + '%'); - - if (currentProgress >= progressLimit || currentProgress >= 100) { - clearInterval(progressTimer); - } } /** diff --git a/phpBB/config/installer/container/services_install_obtain_data.yml b/phpBB/config/installer/container/services_install_obtain_data.yml index 2cfe210309..ecbd3d6d37 100644 --- a/phpBB/config/installer/container/services_install_obtain_data.yml +++ b/phpBB/config/installer/container/services_install_obtain_data.yml @@ -41,3 +41,5 @@ services: parent: installer.module_base arguments: - ["installer.obtain_data.obtain_admin_data", "installer.obtain_data.obtain_database_data", "installer.obtain_data.obtain_server_data", "installer.obtain_data.obtain_email_data", "installer.obtain_data.obtain_board_data", "installer.obtain_data.obtain_imagick_path"] + - true + - false diff --git a/phpBB/config/installer/container/services_install_requirements.yml b/phpBB/config/installer/container/services_install_requirements.yml index 89c517c0cd..b1f38713e4 100644 --- a/phpBB/config/installer/container/services_install_requirements.yml +++ b/phpBB/config/installer/container/services_install_requirements.yml @@ -18,3 +18,5 @@ services: parent: installer.module_base arguments: - ["installer.requirements.check_filesystem", "installer.requirements.check_server_environment"] + - true + - false diff --git a/phpBB/install/exception/invalid_service_name_exception.php b/phpBB/install/exception/invalid_service_name_exception.php new file mode 100644 index 0000000000..e64cd2026f --- /dev/null +++ b/phpBB/install/exception/invalid_service_name_exception.php @@ -0,0 +1,69 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\install\exception; + +class invalid_service_name_exception extends installer_exception +{ + /** + * @var string + */ + private $params; + + /** + * @var string + */ + private $error; + + /** + * Constructor + * + * @param string $error The name of the missing installer module + * @param array $params Additional values for message translation + */ + public function __construct($error, $params = array()) + { + $this->error = $error; + $this->params = $params; + } + + /** + * Returns the language entry's name for the error + * + * @return string + */ + public function get_error() + { + return $this->error; + } + + /** + * Returns parameters for the language entry, if there is any + * + * @return array + */ + public function get_params() + { + return $this->params; + } + + /** + * Returns true, if there are any parameters set + * + * @return bool + */ + public function has_params() + { + return (sizeof($this->params) !== 0); + } +} diff --git a/phpBB/install/exception/module_not_found_exception.php b/phpBB/install/exception/module_not_found_exception.php new file mode 100644 index 0000000000..9fa03fad6e --- /dev/null +++ b/phpBB/install/exception/module_not_found_exception.php @@ -0,0 +1,42 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\install\exception; + +class module_not_found_exception extends installer_exception +{ + /** + * @var string + */ + private $module_service_name; + + /** + * Constructor + * + * @param string $module_service_name The name of the missing installer module + */ + public function __construct($module_service_name) + { + $this->module_service_name = $module_service_name; + } + + /** + * Returns the missing installer module's service name + * + * @return string + */ + public function get_module_service_name() + { + return $this->module_service_name; + } +} diff --git a/phpBB/install/exception/task_not_found_exception.php b/phpBB/install/exception/task_not_found_exception.php new file mode 100644 index 0000000000..11486cc6b0 --- /dev/null +++ b/phpBB/install/exception/task_not_found_exception.php @@ -0,0 +1,42 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\install\exception; + +class task_not_found_exception extends installer_exception +{ + /** + * @var string + */ + private $task_service_name; + + /** + * Constructor + * + * @param string $task_service_name The name of the missing installer module + */ + public function __construct($task_service_name) + { + $this->task_service_name = $task_service_name; + } + + /** + * Returns the missing installer task's service name + * + * @return string + */ + public function get_task_service_name() + { + return $this->task_service_name; + } +} diff --git a/phpBB/install/helper/config.php b/phpBB/install/helper/config.php index f39e92b622..0c04b5e950 100644 --- a/phpBB/install/helper/config.php +++ b/phpBB/install/helper/config.php @@ -232,10 +232,17 @@ class config /** * Increments the task progress + * + * @param int $increment_by The amount to increment by */ - public function increment_current_task_progress() + public function increment_current_task_progress($increment_by = 1) { - $this->progress_data['current_task_progress']++; + $this->progress_data['current_task_progress'] += $increment_by; + + if ($this->progress_data['current_task_progress'] > $this->progress_data['max_task_progress']) + { + $this->progress_data['current_task_progress'] = $this->progress_data['max_task_progress']; + } } /** diff --git a/phpBB/install/installer.php b/phpBB/install/installer.php index cb3dacfbe0..1c7c9c8a92 100644 --- a/phpBB/install/installer.php +++ b/phpBB/install/installer.php @@ -13,15 +13,24 @@ namespace phpbb\install; +use phpbb\install\exception\invalid_service_name_exception; +use phpbb\install\exception\module_not_found_exception; +use phpbb\install\exception\task_not_found_exception; +use phpbb\install\exception\user_interaction_required_exception; +use phpbb\install\helper\config; +use phpbb\install\helper\iohandler\iohandler_interface; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; + class installer { /** - * @var \Symfony\Component\DependencyInjection\ContainerInterface + * @var ContainerInterface */ protected $container; /** - * @var \phpbb\install\helper\config + * @var config */ protected $install_config; @@ -31,17 +40,24 @@ class installer protected $installer_modules; /** - * @var \phpbb\install\helper\iohandler\iohandler_interface + * @var iohandler_interface */ protected $iohandler; + /** + * Stores the number of steps that a given module has + * + * @var array + */ + protected $module_step_count; + /** * Constructor * - * @param \phpbb\install\helper\config $config Installer config handler - * @param \Symfony\Component\DependencyInjection\ContainerInterface $container Dependency injection container + * @param config $config Installer config handler + * @param ContainerInterface $container Dependency injection container */ - public function __construct(\phpbb\install\helper\config $config, \Symfony\Component\DependencyInjection\ContainerInterface $container) + public function __construct(config $config, ContainerInterface $container) { $this->install_config = $config; $this->container = $container; @@ -66,9 +82,9 @@ class installer /** * Sets input-output handler objects * - * @param helper\iohandler\iohandler_interface $iohandler + * @param iohandler_interface $iohandler */ - public function set_iohandler(\phpbb\install\helper\iohandler\iohandler_interface $iohandler) + public function set_iohandler(iohandler_interface $iohandler) { $this->iohandler = $iohandler; } @@ -84,24 +100,44 @@ class installer // Recover install progress $module_index = $this->recover_progress(); - // Count all tasks in the current installer modules - $task_count = 0; - foreach ($this->installer_modules as $name) - { - /** @var \phpbb\install\module_interface $module */ - $module = $this->container->get($name); - - $task_count += $module->get_step_count(); - } - - // Set task count - $this->install_config->set_task_progress_count($task_count); - $this->iohandler->set_task_count($task_count); - + // Variable used to check if the install process have been finished $install_finished = false; + // Flag used by exception handling, whether or not we need to flush output buffer once again + $flush_messages = false; + try { + if ($this->install_config->get_task_progress_count() === 0) + { + // Count all tasks in the current installer modules + $step_count = 0; + foreach ($this->installer_modules as $index => $name) + { + try + { + /** @var \phpbb\install\module_interface $module */ + $module = $this->container->get($name); + } + catch (InvalidArgumentException $e) + { + throw new module_not_found_exception($name); + } + + $module_step_count = $module->get_step_count(); + $step_count += $module_step_count; + $this->module_step_count[$index] = $module_step_count; + } + + // Set task count + $this->install_config->set_task_progress_count($step_count); + } + + // Set up progress information + $this->iohandler->set_task_count( + $this->install_config->get_task_progress_count() + ); + // Run until there are available resources while ($this->install_config->get_time_remaining() > 0 && $this->install_config->get_memory_remaining() > 0) { @@ -117,14 +153,26 @@ class installer $this->install_config->set_active_module($module_service_name, $module_index); // Get module from container - /** @var \phpbb\install\module_interface $module */ - $module = $this->container->get($module_service_name); + try + { + /** @var \phpbb\install\module_interface $module */ + $module = $this->container->get($module_service_name); + } + catch (InvalidArgumentException $e) + { + throw new module_not_found_exception($module_service_name); + } $module_index++; // Check if module should be executed if (!$module->is_essential() && !$module->check_requirements()) { + $this->iohandler->add_log_message(array( + 'SKIP_MODULE', + $module_service_name, + )); + $this->install_config->increment_current_task_progress($this->module_step_count[$module_index - 1]); continue; } @@ -144,10 +192,46 @@ class installer // @todo: Send refresh request } } - catch (\phpbb\install\exception\user_interaction_required_exception $e) + catch (user_interaction_required_exception $e) { // @todo handle exception } + catch (module_not_found_exception $e) + { + $this->iohandler->add_error_message('MODULE_NOT_FOUND', array( + 'MODULE_NOT_FOUND_DESCRIPTION', + $e->get_module_service_name(), + )); + $flush_messages = true; + } + catch (task_not_found_exception $e) + { + $this->iohandler->add_error_message('TASK_NOT_FOUND', array( + 'TASK_NOT_FOUND_DESCRIPTION', + $e->get_task_service_name(), + )); + $flush_messages = true; + } + catch (invalid_service_name_exception $e) + { + if ($e->has_params()) + { + $msg = $e->get_params(); + array_unshift($msg, $e->get_error()); + } + else + { + $msg = $e->get_error(); + } + + $this->iohandler->add_error_message($msg); + $flush_messages = true; + } + + if ($flush_messages) + { + $this->iohandler->send_response(); + } // Save install progress $this->install_config->save_config(); diff --git a/phpBB/install/module/obtain_data/module.php b/phpBB/install/module/obtain_data/module.php index d846593315..a181c5231a 100644 --- a/phpBB/install/module/obtain_data/module.php +++ b/phpBB/install/module/obtain_data/module.php @@ -15,44 +15,6 @@ namespace phpbb\install\module\obtain_data; class module extends \phpbb\install\module_base { - /** - * {@inheritdoc} - */ - public function run() - { - // Recover install progress - $task_index = $this->recover_progress(); - - // Run until there are available resources - while ($this->install_config->get_time_remaining() > 0 && $this->install_config->get_memory_remaining() > 0) - { - // Check if task exists - if (!isset($this->task_collection[$task_index])) - { - break; - } - - // Recover task to be executed - /** @var \phpbb\install\task_interface $task */ - $task = $this->container->get($this->task_collection[$task_index]); - - // Iterate to the next task - $task_index++; - - // Check if we can run the task - if (!$task->is_essential() && !$task->check_requirements()) - { - continue; - } - - $task->run(); - - // Log install progress - $current_task_index = $task_index - 1; - $this->install_config->set_finished_task($this->task_collection[$current_task_index], $current_task_index); - } - } - /** * {@inheritdoc} */ diff --git a/phpBB/install/module/requirements/module.php b/phpBB/install/module/requirements/module.php index f3d1cc71ec..5de6bd70b9 100644 --- a/phpBB/install/module/requirements/module.php +++ b/phpBB/install/module/requirements/module.php @@ -34,8 +34,15 @@ class module extends \phpbb\install\module_base } // Recover task to be executed - /** @var \phpbb\install\task_interface $task */ - $task = $this->container->get($this->task_collection[$task_index]); + try + { + /** @var \phpbb\install\task_interface $task */ + $task = $this->container->get($this->task_collection[$task_index]); + } + catch (InvalidArgumentException $e) + { + throw new task_not_found_exception($this->task_collection[$task_index]); + } // Iterate to the next task $task_index++; diff --git a/phpBB/install/module_base.php b/phpBB/install/module_base.php index 179884c039..ac7ce7583a 100644 --- a/phpBB/install/module_base.php +++ b/phpBB/install/module_base.php @@ -13,23 +13,30 @@ namespace phpbb\install; +use phpbb\install\exception\invalid_service_name_exception; +use phpbb\install\exception\task_not_found_exception; +use phpbb\install\helper\iohandler\iohandler_interface; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; +use phpbb\install\helper\config; + /** * Base class for installer module */ abstract class module_base implements module_interface { /** - * @var \Symfony\Component\DependencyInjection\ContainerInterface + * @var ContainerInterface */ protected $container; /** - * @var \phpbb\install\helper\config + * @var config */ protected $install_config; /** - * @var \phpbb\install\helper\iohandler\iohandler_interface + * @var iohandler_interface */ protected $iohandler; @@ -45,26 +52,33 @@ abstract class module_base implements module_interface */ protected $task_collection; + /** + * @var bool + */ + protected $allow_progress_bar; + /** * Installer module constructor * - * @param array $tasks array of installer tasks for installer module - * @param bool $essential flag that indicates if module is essential or not + * @param array $tasks array of installer tasks for installer module + * @param bool $essential flag indicating whether the module is essential or not + * @param bool $allow_progress_bar flag indicating whether or not to send progress information from within the module */ - public function __construct(array $tasks, $essential = true) + public function __construct(array $tasks, $essential = true, $allow_progress_bar = true) { - $this->task_collection = $tasks; - $this->is_essential = $essential; + $this->task_collection = $tasks; + $this->is_essential = $essential; + $this->allow_progress_bar = $allow_progress_bar; } /** * Dependency getter * - * @param \Symfony\Component\DependencyInjection\ContainerInterface $container - * @param \phpbb\install\helper\config $config - * @param \phpbb\install\helper\iohandler\iohandler_interface $iohandler + * @param ContainerInterface $container + * @param config $config + * @param iohandler_interface $iohandler */ - public function setup(\Symfony\Component\DependencyInjection\ContainerInterface $container, \phpbb\install\helper\config $config, \phpbb\install\helper\iohandler\iohandler_interface $iohandler) + public function setup(ContainerInterface $container, config $config, iohandler_interface $iohandler) { $this->container = $container; $this->install_config = $config; @@ -107,32 +121,55 @@ abstract class module_base implements module_interface } // Recover task to be executed - /** @var \phpbb\install\task_interface $task */ - $task = $this->container->get($this->task_collection[$task_index]); + try + { + /** @var \phpbb\install\task_interface $task */ + $task = $this->container->get($this->task_collection[$task_index]); + } + catch (InvalidArgumentException $e) + { + throw new task_not_found_exception($this->task_collection[$task_index]); + } // Send progress information - $this->iohandler->set_progress( - $task->get_task_lang_name(), - $this->install_config->get_current_task_progress() - ); + if ($this->allow_progress_bar) + { + $this->iohandler->set_progress( + $task->get_task_lang_name(), + $this->install_config->get_current_task_progress() + ); + } // Iterate to the next task $task_index++; - $this->install_config->increment_current_task_progress(); // Check if we can run the task if (!$task->is_essential() && !$task->check_requirements()) { + $this->iohandler->add_log_message(array( + 'SKIP_TASK', + $this->task_collection[$task_index], + )); + $class_name = $this->get_class_from_service_name($this->task_collection[$task_index - 1]); + $this->install_config->increment_current_task_progress($class_name::get_step_count()); continue; } + if ($this->allow_progress_bar) + { + $this->install_config->increment_current_task_progress(); + } + $task->run(); - // Send progress info - $this->iohandler->set_progress( - $task->get_task_lang_name(), - $this->install_config->get_current_task_progress() - ); + // Send progress information + if ($this->allow_progress_bar) + { + $this->iohandler->set_progress( + $task->get_task_lang_name(), + $this->install_config->get_current_task_progress() + ); + } $this->iohandler->send_response(); @@ -179,22 +216,37 @@ abstract class module_base implements module_interface foreach ($this->task_collection as $task_service_name) { - $task_service_name_parts = explode('.', $task_service_name); - - if ($task_service_name_parts[0] !== 'installer') - { - // @todo throw an exception - } - - $class_name = '\\phpbb\\install\\module\\' . $task_service_name_parts[1] . '\\task\\' . $task_service_name_parts[2]; - if (!class_exists($class_name)) - { - // @todo throw an exception - } - + $class_name = $this->get_class_from_service_name($task_service_name); $step_count += $class_name::get_step_count(); } return $step_count; } + + /** + * Returns the name of the class form the service name + * + * @param string $task_service_name Name of the service + * + * @return string Name of the class + * + * @throws invalid_service_name_exception When the service name does not meet the requirements described in task_interface + */ + protected function get_class_from_service_name($task_service_name) + { + $task_service_name_parts = explode('.', $task_service_name); + + if ($task_service_name_parts[0] !== 'installer') + { + throw new invalid_service_name_exception('TASK_SERVICE_INSTALLER_MISSING'); + } + + $class_name = '\\phpbb\\install\\module\\' . $task_service_name_parts[1] . '\\task\\' . $task_service_name_parts[2]; + if (!class_exists($class_name)) + { + throw new invalid_service_name_exception('TASK_CLASS_NOT_FOUND', array($task_service_name, $class_name)); + } + + return $class_name; + } } diff --git a/phpBB/language/en/install.php b/phpBB/language/en/install.php index e3c4b859f0..92251a4992 100644 --- a/phpBB/language/en/install.php +++ b/phpBB/language/en/install.php @@ -249,3 +249,18 @@ $lang = array_merge($lang, array( // Installer general progress messages 'INSTALLER_FINISHED' => 'The installer has finished successfully', )); + +// Installer's general messages +$lang = array_merge($lang, array( + 'MODULE_NOT_FOUND' => 'Module not found', + 'MODULE_NOT_FOUND_DESCRIPTION' => 'No module is found under the service definition “%s” is not defined.', + + 'TASK_NOT_FOUND' => 'Task not found', + 'TASK_NOT_FOUND_DESCRIPTION' => 'No task is found under the service definition “%s” is not defined.', + + 'SKIP_MODULE' => 'Skip “%s” module', + 'SKIP_TASK' => 'Skip “%s” task', + + 'TASK_SERVICE_INSTALLER_MISSING' => 'All installer task services should start with “installer”', + 'TASK_CLASS_NOT_FOUND' => 'Installer task service definition is invalid. Service name “%1$s” given, the expected class namespace is “%2$s” for that. For more information please see the documentation of task_interface.', +));