From c7a986eccdac183cc81b3da486092f4ab82109ba Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 29 Aug 2011 17:17:40 -0400 Subject: [PATCH] [feature/extension-manager] Use an incremental process for enable and purge The enable or purge operation of an extension could take a long time if an expensive operation needs to be executed on a large set of data. To allow this to succeed from a web interface with max_execution_time set in the webserver's php configuration, subsequent requests must continue the operation started earlier. So individual enable and purge implementations must be able to spread their work across multiple steps. PHPBB3-10323 --- phpBB/develop/create_schema_files.php | 1 + phpBB/includes/extension/base.php | 22 +++++- phpBB/includes/extension/interface.php | 39 ++++++++++- phpBB/includes/extension/manager.php | 85 +++++++++++++++++++---- phpBB/install/database_update.php | 1 + phpBB/install/schemas/firebird_schema.sql | 3 +- phpBB/install/schemas/mssql_schema.sql | 3 +- phpBB/install/schemas/mysql_40_schema.sql | 1 + phpBB/install/schemas/mysql_41_schema.sql | 1 + phpBB/install/schemas/oracle_schema.sql | 1 + phpBB/install/schemas/postgres_schema.sql | 3 +- phpBB/install/schemas/sqlite_schema.sql | 3 +- tests/extension/manager_test.php | 10 +++ 13 files changed, 151 insertions(+), 22 deletions(-) diff --git a/phpBB/develop/create_schema_files.php b/phpBB/develop/create_schema_files.php index bc1e28572d..1828719570 100644 --- a/phpBB/develop/create_schema_files.php +++ b/phpBB/develop/create_schema_files.php @@ -1035,6 +1035,7 @@ function get_schema_struct() 'COLUMNS' => array( 'ext_name' => array('VCHAR', ''), 'ext_active' => array('BOOL', 0), + 'ext_state' => array('TEXT', ''), ), 'KEYS' => array( 'ext_name' => array('UNIQUE', 'ext_name'), diff --git a/phpBB/includes/extension/base.php b/phpBB/includes/extension/base.php index 0e6c89491d..8228364d44 100644 --- a/phpBB/includes/extension/base.php +++ b/phpBB/includes/extension/base.php @@ -16,20 +16,38 @@ if (!defined('IN_PHPBB')) } /** +* A base class for extensions without custom enable/disbale/purge code. * * @package extension */ class phpbb_extension_base implements phpbb_extension_interface { - public function enable() + /** + * Single enable step that does nothing + * + * @return false Indicates no further steps are required + */ + public function enable_step($old_state) { + return false; } + /** + * Empty disable method + * + * @return null + */ public function disable() { } - public function purge() + /** + * Single purge step that does nothing + * + * @return false Indicates no further steps are required + */ + public function purge_step($old_state) { + return false; } } diff --git a/phpBB/includes/extension/interface.php b/phpBB/includes/extension/interface.php index 40a5a066a3..7d0ecd72c7 100644 --- a/phpBB/includes/extension/interface.php +++ b/phpBB/includes/extension/interface.php @@ -16,12 +16,47 @@ if (!defined('IN_PHPBB')) } /** +* The interface extension meta classes have to implement to run custom code +* on enable/disable/purge. * * @package extension */ interface phpbb_extension_interface { - public function enable(); + /** + * enable_step is executed on enabling an extension until it returns false. + * + * Calls to this function can be made in subsequent requests, when the + * function is invoked through a webserver with a too low max_execution_time. + * + * @param mixed $old_state The return value of the previous call + * of this method, or false on the first call + * @return mixed Returns false after last step, otherwise + * temporary state which is passed as an + * argument to the next step + */ + public function enable_step($old_state); + + /** + * Disables the extension. + * + * Must be a quick operation, that finishes within max_execution_time. + * + * @return null + */ public function disable(); - public function purge(); + + /** + * purge_step is executed on purging an extension until it returns false. + * + * Calls to this function can be made in subsequent requests, when the + * function is invoked through a webserver with a too low max_execution_time. + * + * @param mixed $old_state The return value of the previous call + * of this method, or false on the first call + * @return mixed Returns false after last step, otherwise + * temporary state which is passed as an + * argument to the next step + */ + public function purge_step($old_state); } diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php index d167bc6847..a1863040d0 100644 --- a/phpBB/includes/extension/manager.php +++ b/phpBB/includes/extension/manager.php @@ -109,28 +109,34 @@ class phpbb_extension_manager } /** - * Enables an extension + * Runs a step of the extension enabling process. * - * Calls the enable method on the extension's meta class to allow it to - * make database changes and execute other initialisation code. + * Allows the exentension to enable in a long running script that works + * in multiple steps across requests. State is kept for the extension + * in the extensions table. * - * @param string $name The extension's name - * @return null + * @param string $name The extension's name + * @return bool Whether another run of enable_step is required */ - public function enable($name) + public function enable_step($name) { // ignore extensions that are already enabled if (isset($this->extensions[$name]) && $this->extensions[$name]['ext_active']) { - return; + return false; } + $old_state = (isset($this->extensions[$name]['ext_state'])) ? unserialize($this->extensions[$name]['ext_state']) : false; + $extension = $this->get_extension($name); - $extension->enable(); + $state = $extension->enable_step($old_state); + + $active = ($state === false); $extension_data = array( - 'ext_name' => $name, - 'ext_active' => true, + 'ext_name' => $name, + 'ext_active' => $active, + 'ext_state' => serialize($state), ); $this->extensions[$name] = $extension_data; @@ -148,6 +154,22 @@ class phpbb_extension_manager ' . $this->db->sql_build_array('INSERT', $extension_data); $this->db->sql_query($sql); } + + return !$active; + } + + /** + * Enables an extension + * + * This method completely enables an extension. But it could be long running + * so never call this in a script that has a max_execution time. + * + * @param string $name The extension's name + * @return null + */ + public function enable($name) + { + while ($this->enable_step($name)); } /** @@ -172,9 +194,10 @@ class phpbb_extension_manager $extension_data = array( 'ext_active' => false, + 'ext_state' => serialize(false), ); $this->extensions[$name]['ext_active'] = false; - ksort($this->extensions); + $this->extensions[$name]['ext_state'] = serialize(false); $sql = 'UPDATE ' . $this->extension_table . ' SET ' . $this->db->sql_build_array('UPDATE', $extension_data) . " @@ -191,12 +214,12 @@ class phpbb_extension_manager * @param string $name The extension's name * @return null */ - public function purge($name) + public function purge_step($name) { // ignore extensions that do not exist if (!isset($this->extensions[$name])) { - return; + return false; } // disable first if necessary @@ -205,14 +228,48 @@ class phpbb_extension_manager $this->disable($name); } + $old_state = unserialize($this->extensions[$name]['ext_state']); + $extension = $this->get_extension($name); - $extension->purge(); + $state = $extension->purge_step($old_state); + + // continue until the state is false + if ($state !== false) + { + $extension_data = array( + 'ext_state' => serialize($state), + ); + $this->extensions[$name]['ext_state'] = serialize($state); + + $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); + + return true; + } unset($this->extensions[$name]); $sql = 'DELETE FROM ' . $this->extension_table . " WHERE ext_name = '" . $this->db->sql_escape($name) . "'"; $this->db->sql_query($sql); + + return false; + } + + /** + * Purge an extension + * + * Purges an extension completely at once. This process could run for a while + * so never call this in a script that has a max_execution time. + * + * @param string $name The extension's name + * @return null + */ + public function purge($name) + { + while ($this->purge_step($name)); } /** diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index 45de19c918..9a7231040b 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -1060,6 +1060,7 @@ function database_update_info() 'COLUMNS' => array( 'ext_name' => array('VCHAR', ''), 'ext_active' => array('BOOL', 0), + 'ext_state' => array('TEXT', ''), ), 'KEYS' => array( 'ext_name' => array('UNIQUE', 'ext_name'), diff --git a/phpBB/install/schemas/firebird_schema.sql b/phpBB/install/schemas/firebird_schema.sql index 4952b6bbdf..9bebf11c4b 100644 --- a/phpBB/install/schemas/firebird_schema.sql +++ b/phpBB/install/schemas/firebird_schema.sql @@ -284,7 +284,8 @@ END;; # Table: 'phpbb_ext' CREATE TABLE phpbb_ext ( ext_name VARCHAR(255) CHARACTER SET NONE DEFAULT '' NOT NULL, - ext_active INTEGER DEFAULT 0 NOT NULL + ext_active INTEGER DEFAULT 0 NOT NULL, + ext_state BLOB SUB_TYPE TEXT CHARACTER SET NONE DEFAULT '' NOT NULL );; CREATE UNIQUE INDEX phpbb_ext_ext_name ON phpbb_ext(ext_name);; diff --git a/phpBB/install/schemas/mssql_schema.sql b/phpBB/install/schemas/mssql_schema.sql index 76f3fc6491..bf137b89f4 100644 --- a/phpBB/install/schemas/mssql_schema.sql +++ b/phpBB/install/schemas/mssql_schema.sql @@ -363,7 +363,8 @@ GO */ CREATE TABLE [phpbb_ext] ( [ext_name] [varchar] (255) DEFAULT ('') NOT NULL , - [ext_active] [int] DEFAULT (0) NOT NULL + [ext_active] [int] DEFAULT (0) NOT NULL , + [ext_state] [varchar] (8000) DEFAULT ('') NOT NULL ) ON [PRIMARY] GO diff --git a/phpBB/install/schemas/mysql_40_schema.sql b/phpBB/install/schemas/mysql_40_schema.sql index a1b282418d..54504beb1c 100644 --- a/phpBB/install/schemas/mysql_40_schema.sql +++ b/phpBB/install/schemas/mysql_40_schema.sql @@ -195,6 +195,7 @@ CREATE TABLE phpbb_drafts ( CREATE TABLE phpbb_ext ( ext_name varbinary(255) DEFAULT '' NOT NULL, ext_active tinyint(1) UNSIGNED DEFAULT '0' NOT NULL, + ext_state blob NOT NULL, UNIQUE ext_name (ext_name) ); diff --git a/phpBB/install/schemas/mysql_41_schema.sql b/phpBB/install/schemas/mysql_41_schema.sql index 2f2de17dcb..139833b7b9 100644 --- a/phpBB/install/schemas/mysql_41_schema.sql +++ b/phpBB/install/schemas/mysql_41_schema.sql @@ -195,6 +195,7 @@ CREATE TABLE phpbb_drafts ( CREATE TABLE phpbb_ext ( ext_name varchar(255) DEFAULT '' NOT NULL, ext_active tinyint(1) UNSIGNED DEFAULT '0' NOT NULL, + ext_state text NOT NULL, UNIQUE ext_name (ext_name) ) CHARACTER SET `utf8` COLLATE `utf8_bin`; diff --git a/phpBB/install/schemas/oracle_schema.sql b/phpBB/install/schemas/oracle_schema.sql index f0a958d2f6..2f958b835b 100644 --- a/phpBB/install/schemas/oracle_schema.sql +++ b/phpBB/install/schemas/oracle_schema.sql @@ -413,6 +413,7 @@ END; CREATE TABLE phpbb_ext ( ext_name varchar2(255) DEFAULT '' , ext_active number(1) DEFAULT '0' NOT NULL, + ext_state clob DEFAULT '' , CONSTRAINT u_phpbb_ext_name UNIQUE (ext_name) ) / diff --git a/phpBB/install/schemas/postgres_schema.sql b/phpBB/install/schemas/postgres_schema.sql index 956167ff39..86ee044913 100644 --- a/phpBB/install/schemas/postgres_schema.sql +++ b/phpBB/install/schemas/postgres_schema.sql @@ -317,7 +317,8 @@ CREATE INDEX phpbb_drafts_save_time ON phpbb_drafts (save_time); */ CREATE TABLE phpbb_ext ( ext_name varchar(255) DEFAULT '' NOT NULL, - ext_active INT2 DEFAULT '0' NOT NULL CHECK (ext_active >= 0) + ext_active INT2 DEFAULT '0' NOT NULL CHECK (ext_active >= 0), + ext_state varchar(8000) DEFAULT '' NOT NULL ); CREATE UNIQUE INDEX phpbb_ext_ext_name ON phpbb_ext (ext_name); diff --git a/phpBB/install/schemas/sqlite_schema.sql b/phpBB/install/schemas/sqlite_schema.sql index 223dbc3c02..94e192fc0c 100644 --- a/phpBB/install/schemas/sqlite_schema.sql +++ b/phpBB/install/schemas/sqlite_schema.sql @@ -189,7 +189,8 @@ CREATE INDEX phpbb_drafts_save_time ON phpbb_drafts (save_time); # Table: 'phpbb_ext' CREATE TABLE phpbb_ext ( ext_name varchar(255) NOT NULL DEFAULT '', - ext_active INTEGER UNSIGNED NOT NULL DEFAULT '0' + ext_active INTEGER UNSIGNED NOT NULL DEFAULT '0', + ext_state text(65535) NOT NULL DEFAULT '' ); CREATE UNIQUE INDEX phpbb_ext_ext_name ON phpbb_ext (ext_name); diff --git a/tests/extension/manager_test.php b/tests/extension/manager_test.php index 2035264559..70c3543a69 100644 --- a/tests/extension/manager_test.php +++ b/tests/extension/manager_test.php @@ -8,6 +8,8 @@ */ require_once dirname(__FILE__) . '/../mock/cache.php'; +require_once dirname(__FILE__) . '/ext/bar/bar.php'; +require_once dirname(__FILE__) . '/ext/moo/moo.php'; class phpbb_extension_manager_test extends phpbb_database_test_case { @@ -49,10 +51,14 @@ class phpbb_extension_manager_test extends phpbb_database_test_case public function test_enable() { + phpbb_ext_bar::$state = 0; + $this->assertEquals(array('foo'), array_keys($this->extension_manager->all_enabled())); $this->extension_manager->enable('bar'); $this->assertEquals(array('bar', 'foo'), array_keys($this->extension_manager->all_enabled())); $this->assertEquals(array('bar', 'foo', 'moo'), array_keys($this->extension_manager->all_configured())); + + $this->assertEquals(4, phpbb_ext_bar::$state); } public function test_disable() @@ -65,10 +71,14 @@ class phpbb_extension_manager_test extends phpbb_database_test_case public function test_purge() { + phpbb_ext_moo::$purged = false; + $this->assertEquals(array('foo'), array_keys($this->extension_manager->all_enabled())); $this->assertEquals(array('foo', 'moo'), array_keys($this->extension_manager->all_configured())); $this->extension_manager->purge('moo'); $this->assertEquals(array('foo'), array_keys($this->extension_manager->all_enabled())); $this->assertEquals(array('foo'), array_keys($this->extension_manager->all_configured())); + + $this->assertTrue(phpbb_ext_moo::$purged); } }