Merge remote-tracking branch 'EXreaction/feature/migrations-extensions' into develop

# By Nathaniel Guse (6) and Nathan Guse (3)
# Via Nathan Guse (1) and Nathaniel Guse (1)
* EXreaction/feature/migrations-extensions:
  [feature/migrations] Use getLocalisedMessage() function to get error message
  [feature/migrations] Fix failing tests (again)
  [feature/migrations] Fix failing tests
  [feature/migrations] Catch and display errors from the migrator
  [feature/migrations] Call revert correctly when purging an extension
  [feature/migrations] Inject Migrator instead of using the container to fetch
  [feature/migrations] Fix path to extension migrations
  [feature/migrations] Automatically install/revert migrations for extensions
  [feature/migrations] Make the container available to extension installers

phpBB/adm/style/acp_ext_enable.html

@@ -7,7 +7,13 @@
 	<p>{L_EXTENSIONS_EXPLAIN}</p>
 	<p>{L_ENABLE_EXPLAIN}</p>
 
-	<!-- IF PRE -->
+	<!-- IF MIGRATOR_ERROR -->
+	<div class="errorbox">
+		<p><strong>{L_MIGRATION_EXCEPTION_ERROR}</strong></p>
+		<p>{MIGRATOR_ERROR}</p>
+		<p><a href="{U_RETURN}">{L_RETURN}</a></p>
+	</div>
+	<!-- ELSEIF PRE -->
 	<div class="errorbox">
 		<p>{L_ENABLE_CONFIRM}</p>
 	</div>

phpBB/adm/style/acp_ext_purge.html

@@ -7,7 +7,13 @@
 	<p>{L_EXTENSIONS_EXPLAIN}</p>
 	<p>{L_PURGE_EXPLAIN}</p>
 
-	<!-- IF PRE -->
+	<!-- IF MIGRATOR_ERROR -->
+	<div class="errorbox">
+		<p><strong>{L_MIGRATION_EXCEPTION_ERROR}</strong></p>
+		<p>{MIGRATOR_ERROR}</p>
+		<p><a href="{U_RETURN}">{L_RETURN}</a></p>
+	</div>
+	<!-- ELSEIF PRE -->
 	<div class="errorbox">
 		<p>{L_PURGE_CONFIRM}</p>
 	</div>

phpBB/config/services.yml

@@ -112,8 +112,10 @@ services:
     ext.manager:
         class: phpbb_extension_manager
         arguments:
+            - @service_container
             - @dbal.conn
             - @config
+            - @migrator
             - %tables.ext%
             - %core.root_path%
             - .%core.php_ext%

phpBB/includes/acp/acp_extensions.php

@@ -37,7 +37,7 @@ class acp_extensions
 		$this->template = $template;
 		$this->user = $user;
 
-		$user->add_lang(array('install', 'acp/extensions'));
+		$user->add_lang(array('install', 'acp/extensions', 'migrator'));
 
 		$this->page_title = 'ACP_EXTENSIONS';
 
@@ -103,11 +103,18 @@ class acp_extensions
 					trigger_error($user->lang['EXTENSION_NOT_AVAILABLE'] . adm_back_link($this->u_action));
 				}
 
-				if ($phpbb_extension_manager->enable_step($ext_name))
+				try
 				{
-					$template->assign_var('S_NEXT_STEP', true);
+					if ($phpbb_extension_manager->enable_step($ext_name))
+					{
+						$template->assign_var('S_NEXT_STEP', true);
 
-					meta_refresh(0, $this->u_action . '&action=enable&ext_name=' . urlencode($ext_name));
+						meta_refresh(0, $this->u_action . '&action=enable&ext_name=' . urlencode($ext_name));
+					}
+				}
+				catch (phpbb_db_migration_exception $e)
+				{
+					$template->assign_var('MIGRATOR_ERROR', $e->getLocalisedMessage($user));
 				}
 
 				$this->tpl_name = 'acp_ext_enable';
@@ -156,11 +163,18 @@ class acp_extensions
 			break;
 
 			case 'purge':
-				if ($phpbb_extension_manager->purge_step($ext_name))
+				try
 				{
-					$template->assign_var('S_NEXT_STEP', true);
+					if ($phpbb_extension_manager->purge_step($ext_name))
+					{
+						$template->assign_var('S_NEXT_STEP', true);
 
-					meta_refresh(0, $this->u_action . '&action=purge&ext_name=' . urlencode($ext_name));
+						meta_refresh(0, $this->u_action . '&action=purge&ext_name=' . urlencode($ext_name));
+					}
+				}
+				catch (phpbb_db_migration_exception $e)
+				{
+					$template->assign_var('MIGRATOR_ERROR', $e->getLocalisedMessage($user));
 				}
 
 				$this->tpl_name = 'acp_ext_purge';

phpBB/includes/extension/base.php

@@ -15,6 +15,8 @@ if (!defined('IN_PHPBB'))
 	exit;
 }
 
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
 /**
 * A base class for extensions without custom enable/disable/purge code.
 *
@@ -22,6 +24,19 @@ if (!defined('IN_PHPBB'))
 */
 class phpbb_extension_base implements phpbb_extension_interface
 {
+	/** @var ContainerInterface */
+	protected $container;
+
+	/**
+	* Constructor
+	*
+	* @param ContainerInterface $container Container object
+	*/
+	public function __construct(ContainerInterface $container)
+	{
+		$this->container = $container;
+	}
+
 	/**
 	* Single enable step that does nothing
 	*

phpBB/includes/extension/manager.php

@@ -15,6 +15,8 @@ if (!defined('IN_PHPBB'))
 	exit;
 }
 
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
 /**
 * The extension manager provides means to activate/deactivate extensions.
 *
@@ -22,8 +24,12 @@ if (!defined('IN_PHPBB'))
 */
 class phpbb_extension_manager
 {
+	/** @var ContainerInterface */
+	protected $container;
+
 	protected $db;
 	protected $config;
+	protected $migrator;
 	protected $cache;
 	protected $php_ext;
 	protected $extensions;
@@ -34,6 +40,7 @@ class phpbb_extension_manager
 	/**
 	* Creates a manager and loads information from database
 	*
+	* @param ContainerInterface $container A container
 	* @param phpbb_db_driver $db A database connection
 	* @param phpbb_config $config phpbb_config
 	* @param string $extension_table The name of the table holding extensions
@@ -42,11 +49,13 @@ 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(phpbb_db_driver $db, phpbb_config $config, $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_db_migrator $migrator, $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->php_ext = $php_ext;
 		$this->extension_table = $extension_table;
@@ -126,11 +135,11 @@ class phpbb_extension_manager
 
 		if (class_exists($extension_class_name))
 		{
-			return new $extension_class_name;
+			return new $extension_class_name($this->container);
 		}
 		else
 		{
-			return new phpbb_extension_base;
+			return new phpbb_extension_base($this->container);
 		}
 	}
 
@@ -166,6 +175,12 @@ 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);
 
@@ -317,6 +332,12 @@ 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);
 
@@ -490,4 +511,58 @@ class phpbb_extension_manager
 	{
 		return new phpbb_extension_finder($this, $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)
+	{
+		$migrations_path = $this->phpbb_root_path . $this->get_extension_path($extension_name) . 'migrations/';
+		if (!file_exists($migrations_path) || !is_dir($migrations_path))
+		{
+			return true;
+		}
+
+		$migrations = $this->migrator->load_migrations($migrations_path);
+
+		// 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();
+
+				// 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;
+				}
+			}
+		}
+		else if ($mode == 'purge')
+		{
+			foreach ($migrations as $migration)
+			{
+				while ($this->migrator->migration_state($migration) !== false)
+				{
+					$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 true;
+	}
 }

phpBB/install/database_update.php

@@ -685,12 +685,12 @@ function _write_result($no_updates, $errored, $error_ary)
 
 function _add_modules($modules_to_install)
 {
-	global $phpbb_root_path, $phpEx, $db, $phpbb_extension_manager, $config;
+	global $phpbb_root_path, $phpEx, $db, $phpbb_extension_manager, $config, $phpbb_container;
 
 	// modules require an extension manager
 	if (empty($phpbb_extension_manager))
 	{
-		$phpbb_extension_manager = new phpbb_extension_manager($db, $config, EXT_TABLE, $phpbb_root_path, ".$phpEx");
+		$phpbb_extension_manager = $phpbb_container->get('ext.manager');
 	}
 
 	include_once($phpbb_root_path . 'includes/acp/acp_modules.' . $phpEx);

phpBB/install/install_install.php

@@ -1456,12 +1456,12 @@ class install_install extends module
 	*/
 	function add_modules($mode, $sub)
 	{
-		global $db, $lang, $phpbb_root_path, $phpEx, $phpbb_extension_manager, $config;
+		global $db, $lang, $phpbb_root_path, $phpEx, $phpbb_extension_manager, $config, $phpbb_container;
 
 		// modules require an extension manager
 		if (empty($phpbb_extension_manager))
 		{
-			$phpbb_extension_manager = new phpbb_extension_manager($db, $config, EXT_TABLE, $phpbb_root_path, ".$phpEx");
+			$phpbb_extension_manager = $phpbb_container->get('ext.manager');
 		}
 
 		include_once($phpbb_root_path . 'includes/acp/acp_modules.' . $phpEx);

tests/extension/manager_test.php

@@ -25,14 +25,7 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 	{
 		parent::setUp();
 
-		$this->extension_manager = new phpbb_extension_manager(
+		$this->extension_manager = $this->create_extension_manager();
-			$this->new_dbal(),
-			new phpbb_config(array()),
-			'phpbb_ext',
-			dirname(__FILE__) . '/',
-			'.php',
-			new phpbb_mock_cache
-		);
 	}
 
 	public function test_available()
@@ -89,15 +82,30 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 
 	public function test_enabled_no_cache()
 	{
-		$extension_manager = new phpbb_extension_manager(
+		$extension_manager = $this->create_extension_manager(false);
-			$this->new_dbal(),
-			new phpbb_config(array()),
-			'phpbb_ext',
-			dirname(__FILE__) . '/',
-			'.php'
-		);
 
 		$this->assertEquals(array('foo'), array_keys($extension_manager->all_enabled()));
 	}
 
+	protected function create_extension_manager($with_cache = true)
+	{
+
+		$config = new phpbb_config(array());
+		$db = $this->new_dbal();
+		$db_tools = new phpbb_db_tools($db);
+		$phpbb_root_path = __DIR__ . './../../phpBB/';
+		$php_ext = 'php';
+		$table_prefix = 'phpbb_';
+
+		return new phpbb_extension_manager(
+			new phpbb_mock_container_builder(),
+			$db,
+			$config,
+			new phpbb_db_migrator($config, $db, $db_tools, 'phpbb_migrations', $phpbb_root_path, $php_ext, $table_prefix, array()),
+			'phpbb_ext',
+			dirname(__FILE__) . '/',
+			'.' . $php_ext,
+			($with_cache) ? new phpbb_mock_cache() : null
+		);
+	}
 }

tests/extension/metadata_manager_test.php

@@ -34,9 +34,11 @@ class metadata_manager_test extends phpbb_database_test_case
 			'version'		=> '3.1.0',
 		));
 		$this->db = $this->new_dbal();
+		$this->db_tools = new phpbb_db_tools($this->db);
 		$this->phpbb_root_path = dirname(__FILE__) . '/';
 		$this->phpEx = '.php';
 		$this->user = new phpbb_user();
+		$this->table_prefix = 'phpbb_';
 
 		$this->template = new phpbb_template(
 			$this->phpbb_root_path,
@@ -48,8 +50,10 @@ class metadata_manager_test extends phpbb_database_test_case
 		);
 
 		$this->extension_manager = new phpbb_extension_manager(
+			new phpbb_mock_container_builder(),
 			$this->db,
 			$this->config,
+			new phpbb_db_migrator($this->config, $this->db, $this->db_tools, 'phpbb_migrations', $this->phpbb_root_path, $this->php_ext, $this->table_prefix, array()),
 			'phpbb_ext',
 			$this->phpbb_root_path,
 			$this->phpEx,

tests/test_framework/phpbb_functional_test_case.php

@@ -134,19 +134,20 @@ class phpbb_functional_test_case extends phpbb_test_case
 	{
 		global $phpbb_root_path, $phpEx;
 
-		if (!$this->extension_manager)
+		$config = new phpbb_config(array());
-		{
+		$db = $this->get_db();
-			$this->extension_manager = new phpbb_extension_manager(
+		$db_tools = new phpbb_db_tools($db);
-				$this->get_db(),
-				new phpbb_config(array()),
-				self::$config['table_prefix'] . 'ext',
-				$phpbb_root_path,
-				".$phpEx",
-				$this->get_cache_driver()
-			);
-		}
 
-		return $this->extension_manager;
+		return new phpbb_extension_manager(
+			new phpbb_mock_container_builder(),
+			$db,
+			$config,
+			new phpbb_db_migrator($config, $db, $db_tools, self::$config['table_prefix'] . 'migrations', $phpbb_root_path, $php_ext, self::$config['table_prefix'], array()),
+			self::$config['table_prefix'] . 'ext',
+			dirname(__FILE__) . '/',
+			'.' . $php_ext,
+			$this->get_cache_driver()
+		);
 	}
 
 	static protected function install_board()