[ticket/17507] Adjust module removal procedure

PHPBB-17507

phpBB/phpbb/db/migration/tool/module.php

@@ -377,7 +377,7 @@ class module implements \phpbb\db\migration\tool\tool_interface
 		{
 			if (!$this->exists($class, $parent, $module, true))
 			{
-				return;
+				throw new \phpbb\db\migration\exception('MODULE_NOT_EXIST', $module);
 			}
 
 			$parent_sql = '';

tests/migrations/fixtures/migration_remove_jabber.xml

@@ -84,7 +84,7 @@
 			<value>1</value>
 			<value></value>
 			<value>acp</value>
-			<value>0</value>
+			<value>1</value>
 			<value>48</value>
 			<value>59</value>
 			<value>ACP_CLIENT_COMMUNICATION</value>
@@ -120,7 +120,7 @@
 		</row>
 		<row>
 			<value>121</value>
-			<value>u_sendim	</value>
+			<value>u_sendim</value>
 			<value>1</value>
 			<value>0</value>
 			<value>0</value>

tests/migrations/migration_test_base.php

@@ -0,0 +1,151 @@
+<?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.
+*
+*/
+
+abstract class phpbb_migration_test_base extends phpbb_database_test_case
+{
+	/** @var \phpbb\db\driver\driver_interface */
+	protected $db;
+
+	/** @var \Doctrine\DBAL\Connection */
+	protected $doctrine_db;
+
+	/** @var \phpbb\db\tools\tools_interface */
+	protected $db_tools;
+
+	/** @var \phpbb\db\migrator */
+	protected $migrator;
+
+	/** @var \phpbb\config\config */
+	protected $config;
+
+	/** @var \phpbb\extension\manager */
+	protected $extension_manager;
+
+	/** @var string */
+	protected $fixture;
+
+	/** @var string */
+	protected $migration_class;
+
+	public function getDataSet()
+	{
+		return $this->createXMLDataSet(__DIR__ . $this->fixture);
+	}
+
+	protected function setUp(): void
+	{
+		global $cache, $db, $phpbb_log, $phpbb_root_path, $phpEx, $skip_add_log, $table_prefix, $user;
+
+		parent::setUp();
+
+		// Disable the logs
+		$skip_add_log = true;
+
+		$db = $this->db = $this->new_dbal();
+		$this->doctrine_db = $this->new_doctrine_dbal();
+		$factory = new \phpbb\db\tools\factory();
+		$this->db_tools = $factory->get($this->doctrine_db);
+		$this->db_tools->set_table_prefix($table_prefix);
+		$this->cache = new phpbb_mock_cache();
+		$this->auth = new \phpbb\auth\auth();
+		$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
+		$this->config = new \phpbb\config\db($this->db, $this->cache, 'phpbb_config');
+		$this->config->initialise($this->cache);
+		$cache = $this->cache_service = new \phpbb\cache\service($this->cache, $this->config, $this->db, $phpbb_dispatcher, $phpbb_root_path, $phpEx);
+
+
+		$lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx);
+		$lang = new \phpbb\language\language($lang_loader);
+		$user = $this->user = new \phpbb\user($lang, '\phpbb\datetime');
+
+		$phpbb_log = new \phpbb\log\log($this->db, $this->user, $this->auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE);
+
+		$container = new phpbb_mock_container_builder();
+		$container->set('event_dispatcher', $phpbb_dispatcher);
+
+		$finder_factory = $this->createMock('\phpbb\finder\factory');
+		$this->extension_manager = new \phpbb\extension\manager(
+			$container,
+			$this->db,
+			$this->config,
+			$finder_factory,
+			'phpbb_ext',
+			__DIR__ . '/../../phpBB/',
+			null
+		);
+
+		$module_manager = new \phpbb\module\module_manager($this->cache, $this->db, $this->extension_manager, 'phpbb_modules', $phpbb_root_path, $phpEx);
+
+		$this->tools = array(
+			'config'		=> new \phpbb\db\migration\tool\config($this->config),
+			'config_text'	=> new \phpbb\db\migration\tool\config_text(new \phpbb\config\db_text($this->db, 'phpbb_config_text')),
+			'module'		=> new \phpbb\db\migration\tool\module($this->db, $this->user, $module_manager, 'phpbb_modules'),
+			'permission'	=> new \phpbb\db\migration\tool\permission($this->db, $this->cache_service, $this->auth, $phpbb_root_path, $phpEx),
+		);
+
+		$this->migrator = new \phpbb\db\migrator(
+			$container,
+			$this->config,
+			$this->db,
+			$this->db_tools,
+			'phpbb_migrations',
+			__DIR__ . '/../../phpBB/',
+			'php',
+			'phpbb_',
+			self::get_core_tables(),
+			$this->tools,
+			new \phpbb\db\migration\helper()
+		);
+		$container->set('migrator', $this->migrator);
+
+		$migration = $this->migrator->get_migration($this->migration_class);
+		$depends = $migration->depends_on();
+		$this->migrator->populate_migrations($depends);
+
+		$this->migrator->set_migrations([$this->migration_class]);
+	}
+
+	protected function apply_migration()
+	{
+		while (!$this->migrator->finished())
+		{
+			try
+			{
+				$this->migrator->update();
+			}
+			catch (\phpbb\db\migration\exception $e)
+			{
+				$this->fail('Applying migration error: ' . $e->__toString());
+			}
+		}
+
+		return $this->migrator->finished();
+	}
+
+	protected function revert_migration()
+	{
+		while ($this->migrator->migration_state($this->migration_class))
+		{
+			try
+			{
+				$this->migrator->revert($this->migration_class);
+			}
+			catch (\phpbb\db\migration\exception $e)
+			{
+				$this->fail('Reverting migration error: ' . $e->__toString());
+			}
+		}
+
+		return !$this->migrator->migration_state($this->migration_class);
+	}
+}

tests/migrations/remove_jabber_migration_test.php

@@ -11,96 +11,13 @@
 *
 */
 
-class phpbb_migrations_remove_jabber_migration_test extends phpbb_database_test_case
+require_once __DIR__ . '/migration_test_base.php';
+
+class phpbb_migrations_remove_jabber_migration_test extends phpbb_migration_test_base
 {
-	/** @var \phpbb\db\driver\driver_interface */
-	protected $db;
 
-	/** @var \Doctrine\DBAL\Connection */
+	protected $migration_class = '\phpbb\db\migration\data\v400\remove_jabber';
-	protected $doctrine_db;
+	protected $fixture = '/fixtures/migration_remove_jabber.xml';
-
-	/** @var \phpbb\db\tools\tools_interface */
-	protected $db_tools;
-
-	/** @var \phpbb\db\migrator */
-	protected $migrator;
-
-	/** @var \phpbb\config\config */
-	protected $config;
-
-	/** @var \phpbb\extension\manager */
-	protected $extension_manager;
-
-	public function getDataSet()
-	{
-		return $this->createXMLDataSet(__DIR__.'/fixtures/migration_remove_jabber.xml');
-	}
-
-	protected function setUp(): void
-	{
-		global $cache, $db, $phpbb_log, $phpbb_root_path, $phpEx, $skip_add_log, $table_prefix, $user;
-
-		parent::setUp();
-
-		// Disable the logs
-		$skip_add_log = true;
-
-		$db = $this->db = $this->new_dbal();
-		$this->doctrine_db = $this->new_doctrine_dbal();
-		$factory = new \phpbb\db\tools\factory();
-		$this->db_tools = $factory->get($this->doctrine_db);
-		$this->db_tools->set_table_prefix($table_prefix);
-		$this->cache = new phpbb_mock_cache();
-		$this->auth = new \phpbb\auth\auth();
-		$phpbb_dispatcher = new phpbb_mock_event_dispatcher();
-		$cache = $this->cache_service = new \phpbb\cache\service(new \phpbb\cache\driver\dummy(), new \phpbb\config\config(array()), $this->db, $phpbb_dispatcher, $phpbb_root_path, $phpEx);
-
-		$this->config = new \phpbb\config\db($this->db, $this->cache, 'phpbb_config');
-		$this->config->initialise($this->cache);
-
-		$lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx);
-		$lang = new \phpbb\language\language($lang_loader);
-		$user = $this->user = new \phpbb\user($lang, '\phpbb\datetime');
-
-		$phpbb_log = new \phpbb\log\log($this->db, $this->user, $this->auth, $phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, LOG_TABLE);
-
-		$container = new phpbb_mock_container_builder();
-		$container->set('event_dispatcher', $phpbb_dispatcher);
-
-		$finder_factory = $this->createMock('\phpbb\finder\factory');
-		$this->extension_manager = new \phpbb\extension\manager(
-			$container,
-			$this->db,
-			$this->config,
-			$finder_factory,
-			'phpbb_ext',
-			__DIR__ . '/../../phpBB/',
-			null
-		);
-
-		$module_manager = new \phpbb\module\module_manager($this->cache, $this->db, $this->extension_manager, 'phpbb_modules', $phpbb_root_path, $phpEx);
-
-		$tools = array(
-			new \phpbb\db\migration\tool\config($this->config),
-			new \phpbb\db\migration\tool\module($this->db, $this->user, $module_manager, 'phpbb_modules'),
-			new \phpbb\db\migration\tool\permission($this->db, $this->cache_service, $this->auth, $phpbb_root_path, $phpEx),
-		);
-
-		$this->migrator = new \phpbb\db\migrator(
-			$container,
-			$this->config,
-			$this->db,
-			$this->db_tools,
-			'phpbb_migrations',
-			__DIR__ . '/../../phpBB/',
-			'php',
-			'phpbb_',
-			self::get_core_tables(),
-			$tools,
-			new \phpbb\db\migration\helper()
-		);
-		$container->set('migrator', $this->migrator);
-	}
 
 	public function test_remove_jabber_migration()
 	{
@@ -115,27 +32,16 @@ class phpbb_migrations_remove_jabber_migration_test extends phpbb_database_test_
 			WHERE config_name = 'jab_enable'";
 		$this->assertNotFalse($this->db->sql_query($sql));
 
-		$sql = "SELECT auth_option FROM phpbb_acl_options
+		$this->assertTrue($this->tools['permission']->exists('a_jabber'));
-			WHERE auth_option = 'a_jabber'";
+		$this->assertTrue($this->tools['permission']->exists('u_sendim'));
-		$this->assertNotFalse($this->db->sql_query($sql));
+		$this->assertTrue($this->tools['module']->exists('acp', 'ACP_CLIENT_COMMUNICATION', 'ACP_JABBER_SETTINGS'));
 
-		$this->migrator->set_migrations(['\phpbb\db\migration\data\v400\remove_jabber']);
+		$this->apply_migration();
-
-		while (!$this->migrator->finished())
-		{
-			try
-			{
-				$this->migrator->update();
-			}
-			catch (\phpbb\db\migration\exception $e)
-			{
-				$this->fail($e->__toString());
-			}
-		}
 
 		$sql = "SELECT id FROM phpbb_user_notifications
 			WHERE method = 'notification.method.jabber'";
-		$this->assertFalse($this->db->sql_query($sql));
+		$this->db->sql_query($sql);
+		$this->assertFalse($this->db->sql_fetchfield('id'));
 		
 		$sql = "SELECT id FROM phpbb_user_notifications
 			WHERE method = 'notification.method.email'";
@@ -146,24 +52,22 @@ class phpbb_migrations_remove_jabber_migration_test extends phpbb_database_test_
 
 		$sql = "SELECT config_name FROM phpbb_config
 			WHERE config_name = 'jab_enable'";
-		$this->assertFalse($this->db->sql_query($sql));
+		$this->db->sql_query($sql);
+		$this->assertFalse($this->db->sql_fetchfield('config_name'));
 
-		$sql = "SELECT auth_option FROM phpbb_acl_options
+		$this->assertFalse($this->tools['permission']->exists('a_jabber'));
-			WHERE auth_option = 'a_jabber'";
+		$this->assertFalse($this->tools['permission']->exists('u_sendim'));
-		$this->assertFalse($this->db->sql_query($sql));
+		$this->assertFalse($this->tools['module']->exists('acp', 'ACP_CLIENT_COMMUNICATION', 'ACP_JABBER_SETTINGS'));
 
-
+		$this->revert_migration();
-		while ($this->migrator->migration_state('phpbb_dbal_migration_schema_index'))
-		{
-			$this->migrator->revert('phpbb_dbal_migration_schema_index');
-		}
 
 		$sql = "SELECT config_name FROM phpbb_config
 			WHERE config_name = 'jab_enable'";
-		$this->assertNotFalse($this->db->sql_query($sql));
+		$this->db->sql_query($sql);
+		$this->assertEquals('jab_enable', $this->db->sql_fetchfield('config_name'));
 
-		$sql = "SELECT auth_option FROM phpbb_acl_options
+		$this->assertTrue($this->tools['permission']->exists('a_jabber'));
-			WHERE auth_option = 'a_jabber'";
+		$this->assertTrue($this->tools['permission']->exists('u_sendim'));
-		$this->assertNotFalse($this->db->sql_query($sql));
+		$this->assertTrue($this->tools['module']->exists('acp', 'ACP_CLIENT_COMMUNICATION', 'ACP_JABBER_SETTINGS'));
 	}
 }