Merge pull request #4144 from VSEphpbb/ticket/14434

[ticket/14434] Allow non-migration files inside migrations folder (continued)

* VSEphpbb/ticket/14434:
  [ticket/14434] Refactored to check migrations when setting them
  [ticket/14434] Check migrations in the database updater task
  [ticket/14434] Do not include non-migrations in CLI list
  [ticket/14434] Remove redundant conditional
  [ticket/14434] Fix whitespace mistakes
  [ticket/14434] Remove recursion to simplify is_migration method
  [ticket/14434] Extract migration check to a reusable method
  [ticket/14434] Schema generator should ignore migration helpers

phpBB/phpbb/console/command/db/migration_command.php

@@ -45,7 +45,7 @@ abstract class migration_command extends \phpbb\console\command\command
 
 		$this->migrator->set_migrations($migrations);
 
-		return $migrations;
+		return $this->migrator->get_migrations();
 	}
 
 	protected function finalise_update()

phpBB/phpbb/db/migration/schema_generator.php

@@ -77,8 +77,15 @@ class schema_generator
 		$check_dependencies = true;
 		while (!empty($migrations))
 		{
-			foreach ($migrations as $migration_class)
+			foreach ($migrations as $key => $migration_class)
 			{
+				// Unset classes that are not a valid migration
+				if (\phpbb\db\migrator::is_migration($migration_class) === false)
+				{
+					unset($migrations[$key]);
+					continue;
+				}
+
 				$open_dependencies = array_diff($migration_class::depends_on(), $tree);
 
 				if (empty($open_dependencies))

phpBB/phpbb/db/migrator.php

@@ -170,9 +170,27 @@ class migrator
 	*/
 	public function set_migrations($class_names)
 	{
+		foreach ($class_names as $key => $class)
+		{
+			if (!self::is_migration($class))
+			{
+				unset($class_names[$key]);
+			}
+		}
+
 		$this->migrations = $class_names;
 	}
 
+	/**
+	 * Get the list of available migration class names
+	 *
+	 * @return array Array of all migrations available to be run
+	 */
+	public function get_migrations()
+	{
+		return $this->migrations;
+	}
+
 	/**
 	* Runs a single update step from the next migration to be applied.
 	*
@@ -857,4 +875,27 @@ class migrator
 			));
 		}
 	}
+
+	/**
+	 * Check if a class is a migration.
+	 *
+	 * @param string $migration A migration class name
+	 * @return bool Return true if class is a migration, false otherwise
+	 */
+	static public function is_migration($migration)
+	{
+		if (class_exists($migration))
+		{
+			// Migration classes should extend the abstract class
+			// phpbb\db\migration\migration (which implements the
+			// migration_interface) and be instantiable.
+			$reflector = new \ReflectionClass($migration);
+			if ($reflector->implementsInterface('\phpbb\db\migration\migration_interface') && $reflector->isInstantiable())
+			{
+				return true;
+			}
+		}
+
+		return false;
+	}
 }

phpBB/phpbb/extension/base.php

@@ -73,9 +73,7 @@ class base implements \phpbb\extension\extension_interface
 	*/
 	public function enable_step($old_state)
 	{
-		$migrations = $this->get_migration_file_list();
+		$this->get_migration_file_list();
-
-		$this->migrator->set_migrations($migrations);
 
 		$this->migrator->update();
 
@@ -103,8 +101,6 @@ class base implements \phpbb\extension\extension_interface
 	{
 		$migrations = $this->get_migration_file_list();
 
-		$this->migrator->set_migrations($migrations);
-
 		foreach ($migrations as $migration)
 		{
 			while ($this->migrator->migration_state($migration) !== false)
@@ -137,21 +133,9 @@ class base implements \phpbb\extension\extension_interface
 
 		$migrations = $this->extension_finder->get_classes_from_files($migrations);
 
-		// Unset classes that do not exist or do not extend the
+		$this->migrator->set_migrations($migrations);
-		// abstract class phpbb\db\migration\migration
-		foreach ($migrations as $key => $migration)
-		{
-			if (class_exists($migration))
-			{
-				$reflector = new \ReflectionClass($migration);
-				if ($reflector->implementsInterface('\phpbb\db\migration\migration_interface') && $reflector->isInstantiable())
-				{
-					continue;
-				}
-			}
 
-			unset($migrations[$key]);
+		$migrations = $this->migrator->get_migrations();
-		}
 
 		return $migrations;
 	}

phpBB/phpbb/install/module/update_database/task/update.php

@@ -140,7 +140,7 @@ class update extends task_base
 			->get_classes();
 
 		$this->migrator->set_migrations($migrations);
-		$migration_count = count($migrations);
+		$migration_count = count($this->migrator->get_migrations());
 		$this->iohandler->set_task_count($migration_count, true);
 		$progress_count = $this->installer_config->get('database_update_count', 0);
 

tests/mock/migrator.php

@@ -21,10 +21,6 @@ class phpbb_mock_migrator extends \phpbb\db\migrator
 	{
 	}
 
-	public function set_migrations($class_names)
-	{
-	}
-
 	public function update()
 	{
 	}