[ticket/11386] Send list of migrations instead of using load_migrations

Remove dependency of extension manager for migrator. Keeping load_migrations function for others to use if they desire but requiring the finder be sent to it in order to use it. PHPBB3-11386
author: Nathaniel Guse <nathaniel.guse@gmail.com> 2013-03-03 19:54:22 -0600
committer: Nathaniel Guse <nathaniel.guse@gmail.com> 2013-03-03 19:54:22 -0600
commit: e4f782819968ec44f1dd207dc9de7ec703826d29 (patch)
tree: e55e3aa115c561d0522b15c897a86a2de8d208ef
parent: bee4f8d8185d4ff5278be758db4ea4a814f09b4f (diff)
download: forums-e4f782819968ec44f1dd207dc9de7ec703826d29.tar
forums-e4f782819968ec44f1dd207dc9de7ec703826d29.tar.gz
forums-e4f782819968ec44f1dd207dc9de7ec703826d29.tar.bz2
forums-e4f782819968ec44f1dd207dc9de7ec703826d29.tar.xz
forums-e4f782819968ec44f1dd207dc9de7ec703826d29.zip
10 files changed, 160 insertions, 161 deletions
diff --git a/phpBB/config/migrator.yml b/phpBB/config/migrator.yml
index 42445ef9bf..999a2d41a3 100644
--- a/phpBB/config/migrator.yml
+++ b/phpBB/config/migrator.yml
@@ -10,8 +10,6 @@ services:
             - %core.php_ext%
             - %core.table_prefix%
             - @migrator.tool_collection
-        calls:
-            - [set_extension_manager, [@ext.manager]]
 
     migrator.tool_collection:
         class: phpbb_di_service_collection
diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml
index 250e4a782b..3e4ae8d129 100644
--- a/phpBB/config/services.yml
+++ b/phpBB/config/services.yml
@@ -116,12 +116,11 @@ services:
             - @service_container
             - @dbal.conn
             - @config
+            - @migrator
             - %tables.ext%
             - %core.root_path%
             - .%core.php_ext%
             - @cache.driver
-        calls:
-            - [set_migrator, [@migrator]]
 
     ext.finder:
         class: phpbb_extension_finder
diff --git a/phpBB/includes/db/migrator.php b/phpBB/includes/db/migrator.php
index de9c06948c..b925ca5297 100644
--- a/phpBB/includes/db/migrator.php
+++ b/phpBB/includes/db/migrator.php
@@ -29,10 +29,7 @@ class phpbb_db_migrator
 	protected $db;
 
 	/** @var phpbb_db_tools */
-	protected $db_tools;
-
-	/** @var phpbb_extension_manager */
-	protected $extension_manager;
+	protected $db_tools
 
 	/** @var string */
 	protected $table_prefix;
@@ -94,16 +91,6 @@ class phpbb_db_migrator
 	}
 
 	/**
-	* Set Extension Manager (required)
-	*
-	* Not in constructor to prevent circular reference error
-	*/
-	public function set_extension_manager(phpbb_extension_manager $extension_manager)
-	{
-		$this->extension_manager = $extension_manager;
-	}
-
-	/**
 	* Loads all migrations and their application state from the database.
 	*
 	* @return null
@@ -146,98 +133,6 @@ class phpbb_db_migrator
 	}
 
 	/**
-	* This function adds all migrations in a specified directory to the migrations table
-	*
-	* THIS SHOULD NOT GENERALLY BE USED! THIS IS FOR THE PHPBB INSTALLER.
-	* THIS WILL THROW ERRORS IF MIGRATIONS ALREADY EXIST IN THE TABLE, DO NOT CALL MORE THAN ONCE!
-	*
-	* @param string $path Path to migration data files
-	* @param bool $recursive Set to true to also load data files from subdirectories
-	* @return null
-	*/
-	public function populate_migrations_from_directory($path, $recursive = true)
-	{
-		$existing_migrations = $this->migrations;
-
-		$this->migrations = array();
-		$this->load_migrations($path, true, $recursive);
-
-		foreach ($this->migrations as $name)
-		{
-			if ($this->migration_state($name) === false)
-			{
-				$state = array(
-					'migration_depends_on'	=> $name::depends_on(),
-					'migration_schema_done' => true,
-					'migration_data_done'	=> true,
-					'migration_data_state'	=> '',
-					'migration_start_time'	=> time(),
-					'migration_end_time'	=> time(),
-				);
-				$this->insert_migration($name, $state);
-			}
-		}
-
-		$this->migrations = $existing_migrations;
-	}
-
-	/**
-	* Load migration data files from a directory
-	*
-	* Migration data files loaded with this function MUST contain
-	* 	ONLY ONE class in them (or an exception will be thrown).
-	*
-	* @param string $path Path to migration data files
-	* @param bool $check_fulfillable If TRUE (default), we will check
-	* 	if all of the migrations are fulfillable after loading them.
-	* 	If FALSE, we will not check. You SHOULD check at least once
-	* 	to prevent errors (if including multiple directories, check
-	* 	with the last call to prevent throwing errors unnecessarily).
-	* @return array Array of migration names
-	*/
-	public function load_migrations($path, $check_fulfillable = true)
-	{
-		if (!is_dir($path))
-		{
-			throw new phpbb_db_migration_exception('DIRECTORY INVALID', $path);
-		}
-
-		$migrations = array();
-
-		$finder = $this->extension_manager->get_finder();
-		$files = $finder
-			->extension_directory("/")
-			->find_from_paths(array('/' => $path));
-		foreach ($files as $file)
-		{
-			$migrations[$file['path'] . $file['filename']] = '';
-		}
-		$migrations = $finder->get_classes_from_files($migrations);
-
-		foreach ($migrations as $migration)
-		{
-			if (!in_array($migration, $this->migrations))
-			{
-				$this->migrations[] = $migration;
-			}
-		}
-
-		if ($check_fulfillable)
-		{
-			foreach ($this->migrations as $name)
-			{
-				$unfulfillable = $this->unfulfillable($name);
-				if ($unfulfillable !== false)
-				{
-					throw new phpbb_db_migration_exception('MIGRATION_NOT_FULFILLABLE', $name, $unfulfillable);
-				}
-			}
-		}
-
-		return $this->migrations;
-	}
-
-	/**
 	* Runs a single update step from the next migration to be applied.
 	*
 	* The update step can either be a schema or a (partial) data update. To
@@ -754,4 +649,85 @@ class phpbb_db_migrator
 	{
 		return new $name($this->config, $this->db, $this->db_tools, $this->phpbb_root_path, $this->php_ext, $this->table_prefix);
 	}
+
+	/**
+	* This function adds all migrations sent to it to the migrations table
+	*
+	* THIS SHOULD NOT GENERALLY BE USED! THIS IS FOR THE PHPBB INSTALLER.
+	* THIS WILL THROW ERRORS IF MIGRATIONS ALREADY EXIST IN THE TABLE, DO NOT CALL MORE THAN ONCE!
+	*
+	* @param array $migrations Array of migrations (names) to add to the migrations table
+	* @return null
+	*/
+	public function populate_migrations($migrations)
+	{
+		foreach ($migrations as $name)
+		{
+			if ($this->migration_state($name) === false)
+			{
+				$state = array(
+					'migration_depends_on'	=> $name::depends_on(),
+					'migration_schema_done' => true,
+					'migration_data_done'	=> true,
+					'migration_data_state'	=> '',
+					'migration_start_time'	=> time(),
+					'migration_end_time'	=> time(),
+				);
+				$this->insert_migration($name, $state);
+			}
+		}
+	}
+
+	/**
+	* Load migration data files from a directory
+	*
+	* @param phpbb_extension_finder $finder
+	* @param string $path Path to migration data files
+	* @param bool $check_fulfillable If TRUE (default), we will check
+	* 	if all of the migrations are fulfillable after loading them.
+	* 	If FALSE, we will not check. You SHOULD check at least once
+	* 	to prevent errors (if including multiple directories, check
+	* 	with the last call to prevent throwing errors unnecessarily).
+	* @return array Array of migration names
+	*/
+	public function load_migrations(phpbb_extension_finder $finder, $path, $check_fulfillable = true)
+	{
+		if (!is_dir($path))
+		{
+			throw new phpbb_db_migration_exception('DIRECTORY INVALID', $path);
+		}
+
+		$migrations = array();
+
+		$files = $finder
+			->extension_directory("/")
+			->find_from_paths(array('/' => $path));
+		foreach ($files as $file)
+		{
+			$migrations[$file['path'] . $file['filename']] = '';
+		}
+		$migrations = $finder->get_classes_from_files($migrations);
+
+		foreach ($migrations as $migration)
+		{
+			if (!in_array($migration, $this->migrations))
+			{
+				$this->migrations[] = $migration;
+			}
+		}
+
+		if ($check_fulfillable)
+		{
+			foreach ($this->migrations as $name)
+			{
+				$unfulfillable = $this->unfulfillable($name);
+				if ($unfulfillable !== false)
+				{
+					throw new phpbb_db_migration_exception('MIGRATION_NOT_FULFILLABLE', $name, $unfulfillable);
+				}
+			}
+		}
+
+		return $this->migrations;
+	}
 }
diff --git a/phpBB/includes/extension/manager.php b/phpBB/includes/extension/manager.php
index 0d760681b9..44a30c6280 100644
--- a/phpBB/includes/extension/manager.php
+++ b/phpBB/includes/extension/manager.php
@@ -43,18 +43,20 @@ 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 string $extension_table The name of the table holding extensions
 	* @param string $phpbb_root_path Path to the phpbb includes directory.
 	* @param string $php_ext php file extension
 	* @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, $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;
@@ -69,14 +71,6 @@ class phpbb_extension_manager
 	}
 
 	/**
-	* Set migrator (get around circular reference)
-	*/
-	public function set_migrator(phpbb_db_migrator $migrator)
-	{
-		$this->migrator = $migrator;
-	}
-
-	/**
 	* Loads all extension information from the database
 	*
 	* @return null
@@ -528,13 +522,27 @@ class phpbb_extension_manager
 	*/
 	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))
+		$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;
 		}
 
-		$migrations = $this->migrator->load_migrations($migrations_path);
+		foreach ($file_list as $file)
+		{
+			$migrations[$file['named_path']] = $file['ext_name'];
+		}
+		$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);
diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php
index 4938ef0f87..b84f00659c 100644
--- a/phpBB/install/database_update.php
+++ b/phpBB/install/database_update.php
@@ -210,7 +210,13 @@ if (!$db_tools->sql_table_exists($table_prefix . 'migrations'))
 }
 
 $migrator = $phpbb_container->get('migrator');
-$migrator->load_migrations($phpbb_root_path . 'includes/db/migration/data/');
+$extension_manager = $phpbb_container->get('ext.manager');
+$finder = $extension_manager->get_finder();
+
+$migrations = $finder
+	->core_path('includes/db/migration/data/')
+	->get_classes();
+$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);
diff --git a/phpBB/install/install_install.php b/phpBB/install/install_install.php
index f0280acc40..4cc154509c 100644
--- a/phpBB/install/install_install.php
+++ b/phpBB/install/install_install.php
@@ -114,7 +114,7 @@ class install_install extends module
 				$this->add_bots($mode, $sub);
 				$this->email_admin($mode, $sub);
 				$this->disable_avatars_if_unwritable();
-				$this->populate_migrations($phpbb_container->get('migrator'), $phpbb_root_path);
+				$this->populate_migrations($phpbb_container->get('ext.manager'), $phpbb_container->get('migrator'));
 
 				// Remove the lock file
 				@unlink($phpbb_root_path . 'cache/install_lock');
@@ -1888,12 +1888,17 @@ class install_install extends module
 	* "installs" means it adds all migrations to the migrations table, but does not
 	* perform any of the actions in the migrations.
 	*
+	* @param phpbb_extension_manager $extension_manager
 	* @param phpbb_db_migrator $migrator
-	* @param string $phpbb_root_path
 	*/
-	function populate_migrations($migrator, $phpbb_root_path)
+	function populate_migrations($extension_manager, $migrator)
 	{
-		$migrator->populate_migrations_from_directory($phpbb_root_path . 'includes/db/migration/data/');
+		$finder = $extension_manager->get_finder();
+
+		$migrations = $finder
+			->core_path('includes/db/migration/data/')
+			->get_classes();
+		$migrator->populate_migrations($migrations);
 	}
 
 	/**
diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php
index b447a81cda..89669b85ec 100644
--- a/tests/dbal/migrator_test.php
+++ b/tests/dbal/migrator_test.php
@@ -45,15 +45,6 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
 			new phpbb_db_migration_tool_config($this->config),
 		);
 
-		$this->extension_manager = new phpbb_extension_manager(
-			new phpbb_mock_container_builder(),
-			$this->db,
-			$this->config,
-			'phpbb_ext',
-			dirname(__FILE__) . '/../../phpBB/',
-			'.php',
-			null
-		);
 		$this->migrator = new phpbb_db_migrator(
 			$this->config,
 			$this->db,
@@ -64,7 +55,16 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
 			'phpbb_',
 			$tools
 		);
-		$this->migrator->set_extension_manager($this->extension_manager);
+		$this->extension_manager = new phpbb_extension_manager(
+			new phpbb_mock_container_builder(),
+			$this->db,
+			$this->config,
+			$this->migrator,
+			'phpbb_ext',
+			dirname(__FILE__) . '/../../phpBB/',
+			'.php',
+			null
+		);
 	}
 
 	public function test_update()
diff --git a/tests/extension/manager_test.php b/tests/extension/manager_test.php
index 3b81afc456..1f311116f4 100644
--- a/tests/extension/manager_test.php
+++ b/tests/extension/manager_test.php
@@ -97,15 +97,6 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 		$php_ext = 'php';
 		$table_prefix = 'phpbb_';
 
-		$manager = new phpbb_extension_manager(
-			new phpbb_mock_container_builder(),
-			$db,
-			$config,
-			'phpbb_ext',
-			dirname(__FILE__) . '/',
-			'.' . $php_ext,
-			($with_cache) ? new phpbb_mock_cache() : null
-		);
 		$migrator = new phpbb_db_migrator(
 			$config,
 			$db,
@@ -116,9 +107,15 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 			$table_prefix,
 			array()
 		);
-		$manager->set_migrator($migrator);
-		$migrator->set_extension_manager($manager);
-
-		return $manager;
+		return new phpbb_extension_manager(
+			new phpbb_mock_container_builder(),
+			$db,
+			$config,
+			$migrator,
+			'phpbb_ext',
+			dirname(__FILE__) . '/',
+			'.' . $php_ext,
+			($with_cache) ? new phpbb_mock_cache() : null
+		);
 	}
 }
diff --git a/tests/extension/metadata_manager_test.php b/tests/extension/metadata_manager_test.php
index 7fb19b67e3..081a32e277 100644
--- a/tests/extension/metadata_manager_test.php
+++ b/tests/extension/metadata_manager_test.php
@@ -49,10 +49,21 @@ class metadata_manager_test extends phpbb_database_test_case
 			new phpbb_template_context()
 		);
 
+		$this->migrator = new phpbb_db_migrator(
+			$this->config,
+			$this->db,
+			$this->db_tools,
+			'phpbb_migrations',
+			$this->phpbb_root_path,
+			'php',
+			$this->table_prefix,
+			array()
+		);
 		$this->extension_manager = new phpbb_extension_manager(
 			new phpbb_mock_container_builder(),
 			$this->db,
 			$this->config,
+			$this->migrator,
 			'phpbb_ext',
 			$this->phpbb_root_path,
 			$this->phpEx,
diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php
index 3b9629b9f8..887dfea3b5 100644
--- a/tests/test_framework/phpbb_functional_test_case.php
+++ b/tests/test_framework/phpbb_functional_test_case.php
@@ -138,15 +138,6 @@ class phpbb_functional_test_case extends phpbb_test_case
 		$db = $this->get_db();
 		$db_tools = new phpbb_db_tools($db);
 
-		$extension_manager = new phpbb_extension_manager(
-			new phpbb_mock_container_builder(),
-			$db,
-			$config,
-			self::$config['table_prefix'] . 'ext',
-			dirname(__FILE__) . '/',
-			'.' . $php_ext,
-			$this->get_cache_driver()
-		);
 		$migrator = new phpbb_db_migrator(
 			$config,
 			$db,
@@ -157,8 +148,16 @@ class phpbb_functional_test_case extends phpbb_test_case
 			self::$config['table_prefix'],
 			array()
 		);
-		$extension_manager->set_migrator($migrator);
-		$migrator->set_extension_manager($extension_manager);
+		$extension_manager = new phpbb_extension_manager(
+			new phpbb_mock_container_builder(),
+			$db,
+			$config,
+			$migrator,
+			self::$config['table_prefix'] . 'ext',
+			dirname(__FILE__) . '/',
+			'.' . $php_ext,
+			$this->get_cache_driver()
+		);
 
 		return $extension_manager;
 	}
author	Nathaniel Guse <nathaniel.guse@gmail.com>	2013-03-03 19:54:22 -0600
committer	Nathaniel Guse <nathaniel.guse@gmail.com>	2013-03-03 19:54:22 -0600
commit	e4f782819968ec44f1dd207dc9de7ec703826d29 (patch)
tree	e55e3aa115c561d0522b15c897a86a2de8d208ef
parent	bee4f8d8185d4ff5278be758db4ea4a814f09b4f (diff)
download	forums-e4f782819968ec44f1dd207dc9de7ec703826d29.tar forums-e4f782819968ec44f1dd207dc9de7ec703826d29.tar.gz forums-e4f782819968ec44f1dd207dc9de7ec703826d29.tar.bz2 forums-e4f782819968ec44f1dd207dc9de7ec703826d29.tar.xz forums-e4f782819968ec44f1dd207dc9de7ec703826d29.zip