Merge pull request #3293 from Nicofuma/ticket/13489

[ticket/13489] Disable the event dispatcher in the migrator

phpBB/config/db.yml

@@ -18,6 +18,7 @@ services:
     migrator:
         class: phpbb\db\migrator
         arguments:
+            - @service_container
             - @config
             - @dbal.conn
             - @dbal.tools

phpBB/phpbb/db/migration/container_aware_migration.php

@@ -0,0 +1,36 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @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\db\migration;
+
+use Symfony\Component\DependencyInjection\ContainerAwareInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+* Abstract base class for container aware database migrations.
+*/
+abstract class container_aware_migration extends migration implements ContainerAwareInterface
+{
+	/**
+	 * @var ContainerInterface
+	 */
+	protected $container;
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function setContainer(ContainerInterface $container = null)
+	{
+		$this->container = $container;
+	}
+}

phpBB/phpbb/db/migration/data/v30x/release_3_0_5_rc1.php

@@ -13,7 +13,9 @@
 
 namespace phpbb\db\migration\data\v30x;
 
-class release_3_0_5_rc1 extends \phpbb\db\migration\migration
+use phpbb\db\migration\container_aware_migration;
+
+class release_3_0_5_rc1 extends container_aware_migration
 {
 	public function effectively_installed()
 	{
@@ -55,9 +57,7 @@ class release_3_0_5_rc1 extends \phpbb\db\migration\migration
 
 	public function hash_old_passwords()
 	{
-		global $phpbb_container;
+		$passwords_manager = $this->container->get('passwords.manager');
-
-		$passwords_manager = $phpbb_container->get('passwords.manager');
 		$sql = 'SELECT user_id, user_password
 				FROM ' . $this->table_prefix . 'users
 				WHERE user_pass_convert = 1';

phpBB/phpbb/db/migration/data/v310/soft_delete_mod_convert.php

@@ -13,12 +13,14 @@
 
 namespace phpbb\db\migration\data\v310;
 
+use phpbb\db\migration\container_aware_migration;
+
 /**
  * Migration to convert the Soft Delete MOD for 3.0
  *
  * https://www.phpbb.com/customise/db/mod/soft_delete/
  */
-class soft_delete_mod_convert extends \phpbb\db\migration\migration
+class soft_delete_mod_convert extends container_aware_migration
 {
 	static public function depends_on()
 	{
@@ -115,19 +117,11 @@ class soft_delete_mod_convert extends \phpbb\db\migration\migration
 		}
 	}
 
+	/**
+	 * @return \phpbb\content_visibility
+	 */
 	protected function get_content_visibility()
 	{
-		return new \phpbb\content_visibility(
+		return $this->container->get('content.visibility');
-			new \phpbb\auth\auth(),
-			$this->config,
-			$this->db,
-			new \phpbb\user('\phpbb\datetime'),
-			$this->phpbb_root_path,
-			$this->php_ext,
-			$this->table_prefix . 'forums',
-			$this->table_prefix . 'posts',
-			$this->table_prefix . 'topics',
-			$this->table_prefix . 'users'
-		);
 	}
 }

phpBB/phpbb/db/migration/profilefield_base_migration.php

@@ -13,7 +13,7 @@
 
 namespace phpbb\db\migration;
 
-abstract class profilefield_base_migration extends \phpbb\db\migration\migration
+abstract class profilefield_base_migration extends container_aware_migration
 {
 	protected $profilefield_name;
 
@@ -237,8 +237,7 @@ abstract class profilefield_base_migration extends \phpbb\db\migration\migration
 
 		if ($profile_row === null)
 		{
-			global $phpbb_container;
+			$manager = $this->container->get('profilefields.manager');
-			$manager = $phpbb_container->get('profilefields.manager');
 			$profile_row = $manager->build_insert_sql_array(array());
 		}
 

phpBB/phpbb/db/migrator.php

@@ -13,11 +13,19 @@
 
 namespace phpbb\db;
 
+use Symfony\Component\DependencyInjection\ContainerAwareInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
 /**
 * The migrator is responsible for applying new migrations in the correct order.
 */
 class migrator
 {
+	/**
+	 * @var ContainerInterface
+	 */
+	protected $container;
+
 	/** @var \phpbb\config\config */
 	protected $config;
 
@@ -77,15 +85,16 @@ class migrator
 	/**
 	 * The output handler. A null handler is configured by default.
 	 *
-	 * @var migrator_output_handler
+	 * @var migrator_output_handler_interface
 	 */
 	public $output_handler;
 
 	/**
 	* Constructor of the database migrator
 	*/
-	public function __construct(\phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, \phpbb\db\tools $db_tools, $migrations_table, $phpbb_root_path, $php_ext, $table_prefix, $tools, \phpbb\db\migration\helper $helper)
+	public function __construct(ContainerInterface $container, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, \phpbb\db\tools $db_tools, $migrations_table, $phpbb_root_path, $php_ext, $table_prefix, $tools, \phpbb\db\migration\helper $helper)
 	{
+		$this->container = $container;
 		$this->config = $config;
 		$this->db = $db;
 		$this->db_tools = $db_tools;
@@ -171,6 +180,18 @@ class migrator
 	* @return null
 	*/
 	public function update()
+	{
+		$this->container->get('dispatcher')->disable();
+		$this->update_do();
+		$this->container->get('dispatcher')->enable();
+	}
+
+	/**
+	 * Effectively runs a single update step from the next migration to be applied.
+	 *
+	 * @return null
+	 */
+	protected function update_do()
 	{
 		foreach ($this->migrations as $name)
 		{
@@ -317,7 +338,7 @@ class migrator
 			catch (\phpbb\db\migration\exception $e)
 			{
 				// Revert the schema changes
-				$this->revert($name);
+				$this->revert_do($name);
 
 				// Rethrow exception
 				throw $e;
@@ -337,9 +358,21 @@ class migrator
 	* check if revert() needs to be called again use the migration_state() method.
 	*
 	* @param string $migration String migration name to revert (including any that depend on this migration)
-	* @return null
 	*/
 	public function revert($migration)
+	{
+		$this->container->get('dispatcher')->disable();
+		$this->revert_do($migration);
+		$this->container->get('dispatcher')->enable();
+	}
+
+	/**
+	 * Effectively runs a single revert step from the last migration installed
+	 *
+	 * @param string $migration String migration name to revert (including any that depend on this migration)
+	 * @return null
+	 */
+	protected function revert_do($migration)
 	{
 		if (!isset($this->migration_state[$migration]))
 		{
@@ -351,7 +384,7 @@ class migrator
 		{
 			if (!empty($state['migration_depends_on']) && in_array($migration, $state['migration_depends_on']))
 			{
-				$this->revert($name);
+				$this->revert_do($name);
 			}
 		}
 
@@ -742,7 +775,14 @@ class migrator
 	*/
 	protected function get_migration($name)
 	{
-		return new $name($this->config, $this->db, $this->db_tools, $this->phpbb_root_path, $this->php_ext, $this->table_prefix);
+		$migration = new $name($this->config, $this->db, $this->db_tools, $this->phpbb_root_path, $this->php_ext, $this->table_prefix);
+
+		if ($migration instanceof ContainerAwareInterface)
+		{
+			$migration->setContainer($this->container);
+		}
+
+		return $migration;
 	}
 
 	/**

phpBB/phpbb/event/dispatcher.php

@@ -14,6 +14,7 @@
 namespace phpbb\event;
 
 use Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher;
+use Symfony\Component\EventDispatcher\Event;
 
 /**
 * Extension of the Symfony2 EventDispatcher
@@ -31,6 +32,11 @@ use Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher;
 */
 class dispatcher extends ContainerAwareEventDispatcher implements dispatcher_interface
 {
+	/**
+	 * @var bool
+	 */
+	protected $disabled = false;
+
 	/**
 	* {@inheritdoc}
 	*/
@@ -40,4 +46,33 @@ class dispatcher extends ContainerAwareEventDispatcher implements dispatcher_int
 		$this->dispatch($eventName, $event);
 		return $event->get_data_filtered(array_keys($data));
 	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function dispatch($eventName, Event $event = null)
+	{
+		if ($this->disabled)
+		{
+			return $event;
+		}
+
+		return parent::dispatch($eventName, $event);
+	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function disable()
+	{
+		$this->disabled = true;
+	}
+
+	/**
+	 * {@inheritdoc}
+	 */
+	public function enable()
+	{
+		$this->disabled = false;
+	}
 }

phpBB/phpbb/event/dispatcher_interface.php

@@ -37,4 +37,14 @@ interface dispatcher_interface extends \Symfony\Component\EventDispatcher\EventD
 	* @return mixed
 	*/
 	public function trigger_event($eventName, $data = array());
+
+	/**
+	 * Disable the event dispatcher.
+	 */
+	public function disable();
+
+	/**
+	 * Enable the event dispatcher.
+	 */
+	public function enable();
 }

tests/dbal/migrator_test.php

@@ -46,7 +46,10 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
 			new \phpbb\db\migration\tool\config($this->config),
 		);
 
+		$container = new phpbb_mock_container_builder();
+
 		$this->migrator = new \phpbb\db\migrator(
+			$container,
 			$this->config,
 			$this->db,
 			$this->db_tools,
@@ -57,9 +60,8 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
 			$tools,
 			new \phpbb\db\migration\helper()
 		);
-
+		$container->set('migrator', $this->migrator);
-		$container = new phpbb_mock_container_builder();
+		$container->set('dispatcher', new phpbb_mock_event_dispatcher());
-		$container->set('migrator', $migrator);
 		$user = new \phpbb\user('\phpbb\datetime');
 
 		$this->extension_manager = new \phpbb\extension\manager(

tests/extension/manager_test.php

@@ -156,7 +156,10 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 		$table_prefix = 'phpbb_';
 		$user = new \phpbb\user('\phpbb\user');
 
+		$container = new phpbb_mock_container_builder();
+
 		$migrator = new \phpbb\db\migrator(
+			$container,
 			$config,
 			$db,
 			$db_tools,
@@ -167,7 +170,6 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 			array(),
 			new \phpbb\db\migration\helper()
 		);
-		$container = new phpbb_mock_container_builder();
 		$container->set('migrator', $migrator);
 
 		return new \phpbb\extension\manager(

tests/extension/metadata_manager_test.php

@@ -62,7 +62,10 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
 			new \phpbb\template\context()
 		);
 
+		$container = new phpbb_mock_container_builder();
+
 		$this->migrator = new \phpbb\db\migrator(
+			$container,
 			$this->config,
 			$this->db,
 			$this->db_tools,
@@ -73,7 +76,6 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
 			array(),
 			new \phpbb\db\migration\helper()
 		);
-		$container = new phpbb_mock_container_builder();
 		$container->set('migrator', $this->migrator);
 
 		$this->extension_manager = new \phpbb\extension\manager(

tests/test_framework/phpbb_functional_test_case.php

@@ -227,7 +227,9 @@ class phpbb_functional_test_case extends phpbb_test_case
 		$db = $this->get_db();
 		$db_tools = new \phpbb\db\tools($db);
 
+		$container = new phpbb_mock_container_builder();
 		$migrator = new \phpbb\db\migrator(
+			$container,
 			$config,
 			$db,
 			$db_tools,
@@ -238,8 +240,8 @@ class phpbb_functional_test_case extends phpbb_test_case
 			array(),
 			new \phpbb\db\migration\helper()
 		);
-		$container = new phpbb_mock_container_builder();
 		$container->set('migrator', $migrator);
+		$container->set('dispatcher', new phpbb_mock_event_dispatcher());
 		$user = new \phpbb\user('\phpbb\datetime');
 
 		$extension_manager = new \phpbb\extension\manager(