diff --git a/phpBB/includes/db/migration/tool/config.php b/phpBB/includes/db/migration/tool/config.php index e7239436d2..6ae419d5e7 100644 --- a/phpBB/includes/db/migration/tool/config.php +++ b/phpBB/includes/db/migration/tool/config.php @@ -103,4 +103,46 @@ class phpbb_db_migration_tool_config implements phpbb_db_migration_tool_interfac $this->config->delete($config_name); } + + /** + * Reverse an original install action + * + * First argument is the original call to the class (e.g. add, remove) + * After the first argument, send the original arguments to the function in the original call + * + * @return null + */ + public function reverse() + { + $arguments = func_get_args(); + $original_call = array_shift($arguments); + + $call = false; + switch ($original_call) + { + case 'add': + $call = 'remove'; + break; + + case 'remove': + $call = 'add'; + break; + + case 'update_if_equals': + $call = 'update_if_equals'; + + // Set to the original value if the current value is what we compared to originally + $arguments = array( + $arguments[2], + $arguments[1], + $arguments[0], + ); + break; + } + + if ($call) + { + return call_user_func_array(array(&$this, $call), $arguments); + } + } } diff --git a/phpBB/includes/db/migration/tool/interface.php b/phpBB/includes/db/migration/tool/interface.php index 5d10246ba1..ced53b2023 100644 --- a/phpBB/includes/db/migration/tool/interface.php +++ b/phpBB/includes/db/migration/tool/interface.php @@ -20,4 +20,14 @@ interface phpbb_db_migration_tool_interface * @return string short name */ public function get_name(); + + /** + * Reverse an original install action + * + * First argument is the original call to the class (e.g. add, remove) + * After the first argument, send the original arguments to the function in the original call + * + * @return null + */ + public function reverse(); } diff --git a/phpBB/includes/db/migration/tool/module.php b/phpBB/includes/db/migration/tool/module.php index 70a246849a..561faac552 100644 --- a/phpBB/includes/db/migration/tool/module.php +++ b/phpBB/includes/db/migration/tool/module.php @@ -471,4 +471,35 @@ class phpbb_db_migration_tool_module implements phpbb_db_migration_tool_interfac $this->cache->destroy("_modules_$class"); } } + + /** + * Reverse an original install action + * + * First argument is the original call to the class (e.g. add, remove) + * After the first argument, send the original arguments to the function in the original call + * + * @return null + */ + public function reverse() + { + $arguments = func_get_args(); + $original_call = array_shift($arguments); + + $call = false; + switch ($original_call) + { + case 'add': + $call = 'remove'; + break; + + case 'remove': + $call = 'add'; + break; + } + + if ($call) + { + return call_user_func_array(array(&$this, $call), $arguments); + } + } } diff --git a/phpBB/includes/db/migration/tool/permission.php b/phpBB/includes/db/migration/tool/permission.php index 7b45b24361..a25fdb345e 100644 --- a/phpBB/includes/db/migration/tool/permission.php +++ b/phpBB/includes/db/migration/tool/permission.php @@ -563,4 +563,59 @@ class phpbb_db_migration_tool_permission implements phpbb_db_migration_tool_inte $this->auth->acl_clear_prefetch(); } + + /** + * Reverse an original install action + * + * First argument is the original call to the class (e.g. add, remove) + * After the first argument, send the original arguments to the function in the original call + * + * @return null + */ + public function reverse() + { + $arguments = func_get_args(); + $original_call = array_shift($arguments); + + $call = false; + switch ($original_call) + { + case 'add': + $call = 'remove'; + break; + + case 'remove': + $call = 'add'; + break; + + case 'permission_set': + $call = 'permission_unset'; + break; + + case 'permission_unset': + $call = 'permission_set'; + break; + + case 'role_add': + $call = 'role_remove'; + break; + + case 'role_remove': + $call = 'role_add'; + break; + + case 'role_update': + // Set to the original value if the current value is what we compared to originally + $arguments = array( + $arguments[1], + $arguments[0], + ); + break; + } + + if ($call) + { + return call_user_func_array(array(&$this, $call), $arguments); + } + } } diff --git a/phpBB/includes/db/migrator.php b/phpBB/includes/db/migrator.php index 359c34bf5d..dc2c746559 100644 --- a/phpBB/includes/db/migrator.php +++ b/phpBB/includes/db/migrator.php @@ -347,6 +347,17 @@ class phpbb_db_migrator catch (phpbb_db_migration_exception $e) { // We should try rolling back here + foreach ($steps as $reverse_step) + { + // Reverse the step that was run + $result = $this->run_step($step, false, true); + + // If we've reached the current step we can break because we reversed everything that was run + if ($reverse_step === $step) + { + break; + } + } var_dump($step); echo $e; @@ -364,11 +375,12 @@ class phpbb_db_migrator * * @param mixed $step Data step from migration * @param mixed $last_result Result to pass to the callable (only for 'custom' method) + * @param bool $reverse False to install, True to attempt uninstallation by reversing the call * @return null */ - protected function run_step($step, $last_result = false) + protected function run_step($step, $last_result = false, $reverse = false) { - $callable_and_parameters = $this->get_callable_from_step($step, $last_result); + $callable_and_parameters = $this->get_callable_from_step($step, $last_result, $reverse); if ($callable_and_parameters === false) { @@ -386,9 +398,10 @@ class phpbb_db_migrator * * @param mixed $step Data step from migration * @param mixed $last_result Result to pass to the callable (only for 'custom' method) + * @param bool $reverse False to install, True to attempt uninstallation by reversing the call * @return array Array with parameters for call_user_func_array(), 0 is the callable, 1 is parameters */ - protected function get_callable_from_step($step, $last_result = false) + protected function get_callable_from_step($step, $last_result = false, $reverse = false) { $type = $step[0]; $parameters = $step[1]; @@ -425,14 +438,7 @@ class phpbb_db_migrator $step = $parameters[1]; - $callable_and_parameters = $this->get_callable_from_step($step); - $callable = $callable_and_parameters[0]; - $sub_parameters = $callable_and_parameters[1]; - - return array( - $callable, - $sub_parameters, - ); + return $this->get_callable_from_step($step); break; case 'custom': if (!is_callable($parameters[0])) @@ -462,6 +468,15 @@ class phpbb_db_migrator throw new phpbb_db_migration_exception('MIGRATION_INVALID_DATA_UNDEFINED_METHOD', $step); } + // Attempt to reverse operations + if ($reverse) + { + return array( + array($this->tools[$class], 'reverse'), + array_unshift($parameters, $method), + ); + } + return array( array($this->tools[$class], $method), $parameters, diff --git a/tests/dbal/migrator_tool_config_test.php b/tests/dbal/migrator_tool_config_test.php index 27511519ca..7d582f230b 100644 --- a/tests/dbal/migrator_tool_config_test.php +++ b/tests/dbal/migrator_tool_config_test.php @@ -94,4 +94,31 @@ class phpbb_dbal_migrator_tool_config_test extends phpbb_test_case } $this->assertFalse(isset($this->config['foo'])); } + + public function test_reverse() + { + $this->config->set('foo', 'bar'); + + try + { + $this->tool->reverse('add', 'foo'); + } + catch (Exception $e) + { + $this->fail($e); + } + $this->assertFalse(isset($this->config['foo'])); + + $this->config->set('foo', 'bar'); + + try + { + $this->tool->reverse('update_if_equals', 'test', 'foo', 'bar'); + } + catch (Exception $e) + { + $this->fail($e); + } + $this->assertEquals('test', $this->config['foo']); + } } diff --git a/tests/dbal/migrator_tool_module.php b/tests/dbal/migrator_tool_module.php index 0b57cbfbcb..6937b6f8c5 100644 --- a/tests/dbal/migrator_tool_module.php +++ b/tests/dbal/migrator_tool_module.php @@ -29,10 +29,10 @@ class phpbb_dbal_migrator_tool_module_test extends phpbb_database_test_case $skip_add_log = true; $db = $this->db = $this->new_dbal(); - $this->cache = new phpbb_cache_service(new phpbb_cache_driver_null()); + $this->cache = new phpbb_cache_service(new phpbb_cache_driver_null(), new phpbb_config(array()), $this->db, $phpbb_root_path, $phpEx); $user = $this->user = new phpbb_user(); - $this->tool = new phpbb_db_migration_tool_module($this->db, $this->cache, $this->user, $phpbb_root_path, $phpEx); + $this->tool = new phpbb_db_migration_tool_module($this->db, $this->cache, $this->user, $phpbb_root_path, $phpEx, 'phpbb_modules'); } public function exists_data() @@ -99,7 +99,7 @@ class phpbb_dbal_migrator_tool_module_test extends phpbb_database_test_case try { - $this->tool->add('acp', ACP_NEW_CAT, array( + $this->tool->add('acp', 'ACP_NEW_CAT', array( 'module_basename' => 'acp_new_module', 'module_langname' => 'ACP_NEW_MODULE', 'module_mode' => 'test', @@ -125,4 +125,26 @@ class phpbb_dbal_migrator_tool_module_test extends phpbb_database_test_case } $this->assertEquals(false, $this->tool->exists('acp', 'ACP_CAT', 'ACP_MODULE')); } + + public function test_reverse() + { + try + { + $this->tool->add('acp', 0, 'ACP_NEW_CAT'); + } + catch (Exception $e) + { + $this->fail($e); + } + + try + { + $this->tool->reverse('add', 'acp', 0, 'ACP_NEW_CAT'); + } + catch (Exception $e) + { + $this->fail($e); + } + $this->assertFalse($this->tool->exists('acp', 0, 'ACP_NEW_CAT')); + } } diff --git a/tests/dbal/migrator_tool_permission.php b/tests/dbal/migrator_tool_permission.php index 2229576cd9..438ab2b28e 100644 --- a/tests/dbal/migrator_tool_permission.php +++ b/tests/dbal/migrator_tool_permission.php @@ -26,7 +26,7 @@ class phpbb_dbal_migrator_tool_permission_test extends phpbb_database_test_case parent::setup(); $db = $this->db = $this->new_dbal(); - $cache = $this->cache = new phpbb_cache_service(new phpbb_cache_driver_null()); + $cache = $this->cache = new phpbb_cache_service(new phpbb_cache_driver_null(), new phpbb_config(array()), $this->db, $phpbb_root_path, $phpEx); $this->auth = new phpbb_auth(); $this->tool = new phpbb_db_migration_tool_permission($this->db, $this->cache, $this->auth, $phpbb_root_path, $phpEx); @@ -133,4 +133,27 @@ class phpbb_dbal_migrator_tool_permission_test extends phpbb_database_test_case } catch (Exception $e) {} } + + public function test_reverse() + { + try + { + $this->tool->reverse('remove', 'global_test', true); + } + catch (Exception $e) + { + $this->fail($e); + } + $this->assertTrue($this->tool->exists('global_test', true)); + + try + { + $this->tool->reverse('add', 'global_test', true); + } + catch (Exception $e) + { + $this->fail($e); + } + $this->assertFalse($this->tool->exists('global_test', true)); + } }