From 9db4e856db426c68d0e3055dbcad9754ce65779d Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Wed, 1 May 2013 13:00:43 -0500 Subject: [ticket/11415] Move while loop from ext manager to acp_extensions.php Now enable_step works as it's supposed to (do one step at a time) and less refreshes are required for the user. PHPBB3-11415 --- phpBB/includes/extension/manager.php | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) (limited to 'phpBB/includes/extension') diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php index a1022762b8..d3e9d56501 100644 --- a/phpBB/includes/extension/manager.php +++ b/phpBB/includes/extension/manager.php @@ -546,22 +546,11 @@ class phpbb_extension_manager $migrations = $finder->get_classes_from_files($migrations); $this->migrator->set_migrations($migrations); - // What is a safe limit of execution time? Half the max execution time should be safe. - $safe_time_limit = (ini_get('max_execution_time') / 2); - $start_time = time(); - if ($mode == 'enable') { - while (!$this->migrator->finished()) - { - $this->migrator->update(); + $this->migrator->update(); - // Are we approaching the time limit? If so we want to pause the update and continue after refreshing - if ((time() - $start_time) >= $safe_time_limit) - { - return false; - } - } + return $this->migrator->finished(); } else if ($mode == 'purge') { @@ -571,11 +560,7 @@ class phpbb_extension_manager { $this->migrator->revert($migration); - // Are we approaching the time limit? If so we want to pause the update and continue after refreshing - if ((time() - $start_time) >= $safe_time_limit) - { - return false; - } + return false; } } } -- cgit v1.2.1 From 7ed21cc6f2b7eee305d0c7c00821ef1ce680b77e Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Wed, 1 May 2013 14:02:13 -0500 Subject: [ticket/11415] Move migrator to base extension class from ext.manager PHPBB3-11415 --- phpBB/includes/extension/base.php | 75 +++++++++++++++++++++++++++++-- phpBB/includes/extension/manager.php | 86 +++++++----------------------------- 2 files changed, 86 insertions(+), 75 deletions(-) (limited to 'phpBB/includes/extension') diff --git a/phpBB/includes/extension/base.php b/phpBB/includes/extension/base.php index d51589d719..dc6d771672 100644 --- a/phpBB/includes/extension/base.php +++ b/phpBB/includes/extension/base.php @@ -27,25 +27,42 @@ class phpbb_extension_base implements phpbb_extension_interface /** @var ContainerInterface */ protected $container; + /** @var string */ + protected $extension_name; + + /** @var string */ + protected $extension_path; + /** * Constructor * * @param ContainerInterface $container Container object + * @param string $extension_name Name of this extension (from ext.manager) + * @param string $extension_path Relative path to this extension */ - public function __construct(ContainerInterface $container) + public function __construct(ContainerInterface $container, $extension_name, $extension_path) { $this->container = $container; + + $this->extension_name = $extension_name; + $this->extension_path = $extension_path; } /** - * Single enable step that does nothing + * Single enable step that installs any included migrations * * @param mixed $old_state State returned by previous call of this method * @return false Indicates no further steps are required */ public function enable_step($old_state) { - return false; + $migrations = $this->get_migration_file_list(); + $migrator = $this->container->get('migrator'); + $migrator->set_migrations($migrations); + + $migrator->update(); + + return !$migrator->finished(); } /** @@ -60,13 +77,63 @@ class phpbb_extension_base implements phpbb_extension_interface } /** - * Single purge step that does nothing + * Single purge step that reverts any included and installed migrations * * @param mixed $old_state State returned by previous call of this method * @return false Indicates no further steps are required */ public function purge_step($old_state) { + $migrations = $this->get_migration_file_list(); + $migrator = $this->container->get('migrator'); + $migrator->set_migrations($migrations); + + foreach ($migrations as $migration) + { + while ($migrator->migration_state($migration) !== false) + { + $migrator->revert($migration); + + return true; + } + } + return false; } + + /** + * Get the list of migration files from this extension + * + * @return array + */ + protected function get_migration_file_list() + { + static $migrations = false; + + if ($migrations !== false) + { + return $migrations; + } + + // Only have the finder search in this extension path directory + $extensions = array( + $this->extension_name => $this->extension_path, + ); + + $extension_manager = $this->container->get('ext.manager'); + $finder = $extension_manager->get_finder(); + $migrations = array(); + $file_list = $finder + ->extension_directory('/migrations') + ->find_from_paths($extensions); + + foreach ($file_list as $file) + { + $migrations[$file['named_path']] = $file['ext_name']; + } + + $migrations = $finder->get_classes_from_files($migrations); + + return $migrations; + } } diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php index d3e9d56501..8b8a69f14c 100644 --- a/phpBB/includes/extension/manager.php +++ b/phpBB/includes/extension/manager.php @@ -138,11 +138,11 @@ class phpbb_extension_manager if (class_exists($extension_class_name)) { - return new $extension_class_name($this->container); + return new $extension_class_name($this->container, $name, $this->get_extension_path($name, true)); } else { - return new phpbb_extension_base($this->container); + return new phpbb_extension_base($this->container, $name, $this->get_extension_path($name, true)); } } @@ -178,12 +178,6 @@ class phpbb_extension_manager $old_state = (isset($this->extensions[$name]['ext_state'])) ? unserialize($this->extensions[$name]['ext_state']) : false; - // Returns false if not completed - if (!$this->handle_migrations($name, 'enable')) - { - return true; - } - $extension = $this->get_extension($name); $state = $extension->enable_step($old_state); @@ -199,12 +193,21 @@ class phpbb_extension_manager $this->extensions[$name]['ext_path'] = $this->get_extension_path($extension_data['ext_name']); ksort($this->extensions); - $sql = 'UPDATE ' . $this->extension_table . ' - SET ' . $this->db->sql_build_array('UPDATE', $extension_data) . " + $sql = 'SELECT COUNT(ext_name) as row_count + FROM ' . $this->extension_table . " WHERE ext_name = '" . $this->db->sql_escape($name) . "'"; - $this->db->sql_query($sql); + $result = $this->db->sql_query($sql); + $count = $this->db->sql_fetchfield('row_count'); + $this->db->sql_freeresult($result); - if (!$this->db->sql_affectedrows()) + if ($count) + { + $sql = 'UPDATE ' . $this->extension_table . ' + SET ' . $this->db->sql_build_array('UPDATE', $extension_data) . " + WHERE ext_name = '" . $this->db->sql_escape($name) . "'"; + $this->db->sql_query($sql); + } + else { $sql = 'INSERT INTO ' . $this->extension_table . ' ' . $this->db->sql_build_array('INSERT', $extension_data); @@ -335,12 +338,6 @@ class phpbb_extension_manager $old_state = unserialize($this->extensions[$name]['ext_state']); - // Returns false if not completed - if (!$this->handle_migrations($name, 'purge')) - { - return true; - } - $extension = $this->get_extension($name); $state = $extension->purge_step($old_state); @@ -514,57 +511,4 @@ class phpbb_extension_manager { return new phpbb_extension_finder($this, $this->filesystem, $this->phpbb_root_path, $this->cache, $this->php_ext, $this->cache_name . '_finder'); } - - /** - * Handle installing/reverting migrations - * - * @param string $extension_name Name of the extension - * @param string $mode enable or purge - * @return bool True if completed, False if not completed - */ - protected function handle_migrations($extension_name, $mode) - { - $extensions = array( - $extension_name => $this->phpbb_root_path . $this->get_extension_path($extension_name), - ); - - $finder = $this->get_finder(); - $migrations = array(); - $file_list = $finder - ->extension_directory('/migrations') - ->find_from_paths($extensions); - - if (empty($file_list)) - { - return true; - } - - foreach ($file_list as $file) - { - $migrations[$file['named_path']] = $file['ext_name']; - } - $migrations = $finder->get_classes_from_files($migrations); - $this->migrator->set_migrations($migrations); - - if ($mode == 'enable') - { - $this->migrator->update(); - - return $this->migrator->finished(); - } - else if ($mode == 'purge') - { - foreach ($migrations as $migration) - { - while ($this->migrator->migration_state($migration) !== false) - { - $this->migrator->revert($migration); - - return false; - } - } - } - - return true; - } } -- cgit v1.2.1 From 60e32728393d4258f92f7893f8275889278a995f Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Wed, 1 May 2013 14:09:08 -0500 Subject: [ticket/11415] Remove migrator dependency from extension manager PHPBB3-11415 --- phpBB/includes/extension/manager.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'phpBB/includes/extension') diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php index 8b8a69f14c..b42043748f 100644 --- a/phpBB/includes/extension/manager.php +++ b/phpBB/includes/extension/manager.php @@ -29,7 +29,6 @@ class phpbb_extension_manager protected $db; protected $config; - protected $migrator; protected $cache; protected $php_ext; protected $extensions; @@ -43,7 +42,6 @@ class phpbb_extension_manager * @param ContainerInterface $container A container * @param phpbb_db_driver $db A database connection * @param phpbb_config $config phpbb_config - * @param phpbb_db_migrator $migrator * @param phpbb_filesystem $filesystem * @param string $extension_table The name of the table holding extensions * @param string $phpbb_root_path Path to the phpbb includes directory. @@ -51,13 +49,12 @@ class phpbb_extension_manager * @param phpbb_cache_driver_interface $cache A cache instance or null * @param string $cache_name The name of the cache variable, defaults to _ext */ - public function __construct(ContainerInterface $container, phpbb_db_driver $db, phpbb_config $config, phpbb_db_migrator $migrator, phpbb_filesystem $filesystem, $extension_table, $phpbb_root_path, $php_ext = 'php', phpbb_cache_driver_interface $cache = null, $cache_name = '_ext') + public function __construct(ContainerInterface $container, phpbb_db_driver $db, phpbb_config $config, phpbb_filesystem $filesystem, $extension_table, $phpbb_root_path, $php_ext = 'php', phpbb_cache_driver_interface $cache = null, $cache_name = '_ext') { $this->container = $container; $this->phpbb_root_path = $phpbb_root_path; $this->db = $db; $this->config = $config; - $this->migrator = $migrator; $this->cache = $cache; $this->filesystem = $filesystem; $this->php_ext = $php_ext; -- cgit v1.2.1 From 87b01fc854b22e3839776505486d5a564e671f5f Mon Sep 17 00:00:00 2001 From: Nathaniel Guse Date: Wed, 1 May 2013 15:18:53 -0500 Subject: [ticket/11415] Make migrator/ext.manager dependencies of the base ext class PHPBB3-11415 --- phpBB/includes/extension/base.php | 30 +++++++++++++++++++----------- phpBB/includes/extension/manager.php | 6 ++++-- 2 files changed, 23 insertions(+), 13 deletions(-) (limited to 'phpBB/includes/extension') diff --git a/phpBB/includes/extension/base.php b/phpBB/includes/extension/base.php index dc6d771672..2d22658ff1 100644 --- a/phpBB/includes/extension/base.php +++ b/phpBB/includes/extension/base.php @@ -27,6 +27,12 @@ class phpbb_extension_base implements phpbb_extension_interface /** @var ContainerInterface */ protected $container; + /** @var phpbb_extension_manager */ + protected $extension_manager; + + /** @var phpbb_db_migrator */ + protected $migrator; + /** @var string */ protected $extension_name; @@ -37,12 +43,15 @@ class phpbb_extension_base implements phpbb_extension_interface * Constructor * * @param ContainerInterface $container Container object + * @param phpbb_extension_manager $extension_manager * @param string $extension_name Name of this extension (from ext.manager) * @param string $extension_path Relative path to this extension */ - public function __construct(ContainerInterface $container, $extension_name, $extension_path) + public function __construct(ContainerInterface $container, phpbb_extension_manager $extension_manager, phpbb_db_migrator $migrator, $extension_name, $extension_path) { $this->container = $container; + $this->extension_manager = $extension_manager; + $this->migrator = $migrator; $this->extension_name = $extension_name; $this->extension_path = $extension_path; @@ -57,12 +66,12 @@ class phpbb_extension_base implements phpbb_extension_interface public function enable_step($old_state) { $migrations = $this->get_migration_file_list(); - $migrator = $this->container->get('migrator'); - $migrator->set_migrations($migrations); - $migrator->update(); + $this->migrator->set_migrations($migrations); - return !$migrator->finished(); + $this->migrator->update(); + + return !$this->migrator->finished(); } /** @@ -85,14 +94,14 @@ class phpbb_extension_base implements phpbb_extension_interface public function purge_step($old_state) { $migrations = $this->get_migration_file_list(); - $migrator = $this->container->get('migrator'); - $migrator->set_migrations($migrations); + + $this->migrator->set_migrations($migrations); foreach ($migrations as $migration) { - while ($migrator->migration_state($migration) !== false) + while ($this->migrator->migration_state($migration) !== false) { - $migrator->revert($migration); + $this->migrator->revert($migration); return true; } @@ -120,8 +129,7 @@ class phpbb_extension_base implements phpbb_extension_interface $this->extension_name => $this->extension_path, ); - $extension_manager = $this->container->get('ext.manager'); - $finder = $extension_manager->get_finder(); + $finder = $this->extension_manager->get_finder(); $migrations = array(); $file_list = $finder ->extension_directory('/migrations') diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php index b42043748f..799c8b2418 100644 --- a/phpBB/includes/extension/manager.php +++ b/phpBB/includes/extension/manager.php @@ -133,13 +133,15 @@ class phpbb_extension_manager { $extension_class_name = 'phpbb_ext_' . str_replace('/', '_', $name) . '_ext'; + $migrator = $this->container->get('migrator'); + if (class_exists($extension_class_name)) { - return new $extension_class_name($this->container, $name, $this->get_extension_path($name, true)); + return new $extension_class_name($this->container, $this, $migrator, $name, $this->get_extension_path($name, true)); } else { - return new phpbb_extension_base($this->container, $name, $this->get_extension_path($name, true)); + return new phpbb_extension_base($this->container, $this, $migrator, $name, $this->get_extension_path($name, true)); } } -- cgit v1.2.1 From 27b2bbb8ff9efc6e06cfc5077dc939b1f1f7d0e5 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Fri, 10 May 2013 13:58:55 -0500 Subject: [ticket/11415] Create function in finder find_from_extension PHPBB3-11415 --- phpBB/includes/extension/base.php | 15 ++------------- phpBB/includes/extension/finder.php | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 13 deletions(-) (limited to 'phpBB/includes/extension') diff --git a/phpBB/includes/extension/base.php b/phpBB/includes/extension/base.php index 2d22658ff1..a6c9bbb5bc 100644 --- a/phpBB/includes/extension/base.php +++ b/phpBB/includes/extension/base.php @@ -125,21 +125,10 @@ class phpbb_extension_base implements phpbb_extension_interface } // Only have the finder search in this extension path directory - $extensions = array( - $this->extension_name => $this->extension_path, - ); - $finder = $this->extension_manager->get_finder(); - $migrations = array(); - $file_list = $finder + $migrations = $finder ->extension_directory('/migrations') - ->find_from_paths($extensions); - - foreach ($file_list as $file) - { - $migrations[$file['named_path']] = $file['ext_name']; - } - + ->find_from_extension($this->extension_name, $this->extension_path); $migrations = $finder->get_classes_from_files($migrations); return $migrations; diff --git a/phpBB/includes/extension/finder.php b/phpBB/includes/extension/finder.php index 766b9e9b63..49bb2a514f 100644 --- a/phpBB/includes/extension/finder.php +++ b/phpBB/includes/extension/finder.php @@ -377,6 +377,34 @@ class phpbb_extension_finder return $files; } + + /** + * Finds all file system entries matching the configured options for one + * specific extension + * + * @param string $extension_name Name of the extension + * @param string $extension_path Relative path to the extension root directory + * @param bool $cache Whether the result should be cached + * @param bool $is_dir Directories will be returned when true, only files + * otherwise + * @return array An array of paths to found items + */ + public function find_from_extension($extension_name, $extension_path, $cache = true, $is_dir = false) + { + $extensions = array( + $extension_name => $extension_path, + ); + + $files = array(); + $file_list = $this->find_from_paths($extensions, $cache, $is_dir); + + foreach ($file_list as $file) + { + $files[$file['named_path']] = $file['ext_name']; + } + + return $files; + } /** * Finds all file system entries matching the configured options from -- cgit v1.2.1 From f91f8666fd423c3e9f74d05355c8fa3e8ece2de9 Mon Sep 17 00:00:00 2001 From: Nathan Guse Date: Fri, 10 May 2013 14:01:31 -0500 Subject: [ticket/11415] Send the extension base the finder rather than the manager PHPBB3-11415 --- phpBB/includes/extension/base.php | 15 +++++++-------- phpBB/includes/extension/manager.php | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) (limited to 'phpBB/includes/extension') diff --git a/phpBB/includes/extension/base.php b/phpBB/includes/extension/base.php index a6c9bbb5bc..c4462b64d8 100644 --- a/phpBB/includes/extension/base.php +++ b/phpBB/includes/extension/base.php @@ -27,8 +27,8 @@ class phpbb_extension_base implements phpbb_extension_interface /** @var ContainerInterface */ protected $container; - /** @var phpbb_extension_manager */ - protected $extension_manager; + /** @var phpbb_extension_finder */ + protected $finder; /** @var phpbb_db_migrator */ protected $migrator; @@ -43,14 +43,14 @@ class phpbb_extension_base implements phpbb_extension_interface * Constructor * * @param ContainerInterface $container Container object - * @param phpbb_extension_manager $extension_manager + * @param phpbb_extension_finder $extension_finder * @param string $extension_name Name of this extension (from ext.manager) * @param string $extension_path Relative path to this extension */ - public function __construct(ContainerInterface $container, phpbb_extension_manager $extension_manager, phpbb_db_migrator $migrator, $extension_name, $extension_path) + public function __construct(ContainerInterface $container, phpbb_extension_finder $extension_finder, phpbb_db_migrator $migrator, $extension_name, $extension_path) { $this->container = $container; - $this->extension_manager = $extension_manager; + $this->extension_finder = $extension_finder; $this->migrator = $migrator; $this->extension_name = $extension_name; @@ -125,11 +125,10 @@ class phpbb_extension_base implements phpbb_extension_interface } // Only have the finder search in this extension path directory - $finder = $this->extension_manager->get_finder(); - $migrations = $finder + $migrations = $this->extension_finder ->extension_directory('/migrations') ->find_from_extension($this->extension_name, $this->extension_path); - $migrations = $finder->get_classes_from_files($migrations); + $migrations = $this->extension_finder->get_classes_from_files($migrations); return $migrations; } diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php index 799c8b2418..48b72bcdd0 100644 --- a/phpBB/includes/extension/manager.php +++ b/phpBB/includes/extension/manager.php @@ -137,11 +137,11 @@ class phpbb_extension_manager if (class_exists($extension_class_name)) { - return new $extension_class_name($this->container, $this, $migrator, $name, $this->get_extension_path($name, true)); + return new $extension_class_name($this->container, $this->get_finder(), $migrator, $name, $this->get_extension_path($name, true)); } else { - return new phpbb_extension_base($this->container, $this, $migrator, $name, $this->get_extension_path($name, true)); + return new phpbb_extension_base($this->container, $this->get_finder(), $migrator, $name, $this->get_extension_path($name, true)); } } -- cgit v1.2.1