From 9fb649793de65a598615c542861281ff15a60439 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Wed, 10 Aug 2016 14:32:00 +0200 Subject: [ticket/14742] Pause after each schema change It is certainly better than running them all at once PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb/db') diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 7fc3e787e2..739959d7b7 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -308,7 +308,14 @@ class migrator $state['migration_data_state'] = ($result === true) ? '' : $result; $state['migration_schema_done'] = ($result === true); - $this->output_handler->write(array('MIGRATION_SCHEMA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); + if ($state['migration_schema_done']) + { + $this->output_handler->write(array('MIGRATION_SCHEMA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); + } + else + { + $this->output_handler->write(array('MIGRATION_SCHEMA_IN_PROGRESS', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_VERY_VERBOSE); + } } else if (!$state['migration_data_done']) { @@ -493,8 +500,10 @@ class migrator try { // Result will be null or true if everything completed correctly + // After any schema update step we allow to pause, since + // database changes can take quite some time $result = $this->run_step($step, $last_result, $revert); - if ($result !== null && $result !== true) + if ($result !== null && $result !== true && strpos($step[0], 'dbtools') !== 0) { return serialize(array( 'result' => $result, -- cgit v1.2.1 From 8e1461ca61e3f452935a1253d3afe65e7322d6bc Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Wed, 10 Aug 2016 14:55:39 +0200 Subject: [ticket/14742] Avoid loop while reverting data This combines reverted updata_data and revert_data into a single array. PHPBB3-14742 --- phpBB/phpbb/db/migration/helper.php | 32 +++++++++++++++++++++++++++ phpBB/phpbb/db/migration/tool/config.php | 5 +++++ phpBB/phpbb/db/migration/tool/config_text.php | 5 +++++ phpBB/phpbb/db/migration/tool/module.php | 5 +++++ phpBB/phpbb/db/migration/tool/permission.php | 5 +++++ phpBB/phpbb/db/migrator.php | 23 +++++++++---------- 6 files changed, 63 insertions(+), 12 deletions(-) (limited to 'phpBB/phpbb/db') diff --git a/phpBB/phpbb/db/migration/helper.php b/phpBB/phpbb/db/migration/helper.php index e40deeb37b..bce2efff51 100644 --- a/phpBB/phpbb/db/migration/helper.php +++ b/phpBB/phpbb/db/migration/helper.php @@ -81,4 +81,36 @@ class helper return $steps; } + + /** + * Reverse the update steps from an array of data changes + * + * 'If' statements and custom methods will be skipped, for all + * other calls the reverse method of the tool class will be called + * + * @param array $steps Update changes from migration + * + * @return array + */ + public function reverse_update_data($steps) + { + $reversed_array = array(); + + foreach ($steps as $step) + { + $parts = explode('.', $step[0]); + $parameters = $step[1]; + + $class = $parts[0]; + $method = isset($parts[1]) ? $parts[1] : false; + + if ($class !== 'if' && $class !== 'custom') + { + array_unshift($parameters, $method); + $reversed_array[] = array($class . '.reverse', $parameters); + } + } + + return array_reverse($reversed_array); + } } diff --git a/phpBB/phpbb/db/migration/tool/config.php b/phpBB/phpbb/db/migration/tool/config.php index f93e7118c4..2a76409db5 100644 --- a/phpBB/phpbb/db/migration/tool/config.php +++ b/phpBB/phpbb/db/migration/tool/config.php @@ -150,6 +150,11 @@ class config implements \phpbb\db\migration\tool\tool_interface $arguments[0], ); break; + + case 'reverse': + // It's like double negative + $call = array_shift($arguments); + break; } if ($call) diff --git a/phpBB/phpbb/db/migration/tool/config_text.php b/phpBB/phpbb/db/migration/tool/config_text.php index bf8ac55023..21b8a8b3a0 100644 --- a/phpBB/phpbb/db/migration/tool/config_text.php +++ b/phpBB/phpbb/db/migration/tool/config_text.php @@ -115,6 +115,11 @@ class config_text implements \phpbb\db\migration\tool\tool_interface $arguments[] = ''; } break; + + case 'reverse': + // It's like double negative + $call = array_shift($arguments); + break; } if ($call) diff --git a/phpBB/phpbb/db/migration/tool/module.php b/phpBB/phpbb/db/migration/tool/module.php index 035625b095..a6baf40dc1 100644 --- a/phpBB/phpbb/db/migration/tool/module.php +++ b/phpBB/phpbb/db/migration/tool/module.php @@ -454,6 +454,11 @@ class module implements \phpbb\db\migration\tool\tool_interface case 'remove': $call = 'add'; break; + + case 'reverse': + // It's like double negative + $call = array_shift($arguments); + break; } if ($call) diff --git a/phpBB/phpbb/db/migration/tool/permission.php b/phpBB/phpbb/db/migration/tool/permission.php index ceff6d7d5a..3a1e2e344b 100644 --- a/phpBB/phpbb/db/migration/tool/permission.php +++ b/phpBB/phpbb/db/migration/tool/permission.php @@ -637,6 +637,11 @@ class permission implements \phpbb\db\migration\tool\tool_interface $arguments[0], ); break; + + case 'reverse': + // It's like double negative + $call = array_shift($arguments); + break; } if ($call) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 739959d7b7..3e69d9613e 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -423,19 +423,11 @@ class migrator if ($state['migration_data_done']) { - if ($state['migration_data_state'] !== 'revert_data') - { - $result = $this->process_data_step($migration->update_data(), $state['migration_data_state'], true); - - $state['migration_data_state'] = ($result === true) ? 'revert_data' : $result; - } - else - { - $result = $this->process_data_step($migration->revert_data(), '', false); + $steps = array_merge($this->helper->reverse_update_data($migration->update_data()), $migration->revert_data()); + $result = $this->process_data_step($steps, $state['migration_data_state']); - $state['migration_data_state'] = ($result === true) ? '' : $result; - $state['migration_data_done'] = ($result === true) ? false : true; - } + $state['migration_data_state'] = ($result === true) ? '' : $result; + $state['migration_data_done'] = ($result === true) ? false : true; $this->set_migration_state($name, $state); } @@ -596,6 +588,13 @@ class migrator throw new \phpbb\db\migration\exception('MIGRATION_INVALID_DATA_MISSING_STEP', $step); } + if ($reverse) + { + // We might get unexpected results when trying + // to revert this, so just avoid it + return false; + } + $condition = $parameters[0]; if (!$condition) -- cgit v1.2.1 From a277f9cf07f73e5f4563d248d1bb4eed49a7e4c9 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Wed, 10 Aug 2016 14:57:33 +0200 Subject: [ticket/14742] Small fixes to migrator PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'phpBB/phpbb/db') diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 3e69d9613e..8637f71414 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -87,7 +87,7 @@ class migrator * * @var migrator_output_handler_interface */ - public $output_handler; + protected $output_handler; /** * Constructor of the database migrator @@ -333,7 +333,7 @@ class migrator $state['migration_data_done'] = ($result === true); $state['migration_end_time'] = ($result === true) ? time() : 0; - if ($state['migration_schema_done']) + if ($state['migration_data_done']) { $this->output_handler->write(array('MIGRATION_DATA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL); } @@ -347,7 +347,6 @@ class migrator // Revert the schema changes $this->revert_do($name); - // Rethrow exception throw $e; } } @@ -518,7 +517,6 @@ class migrator $result = $this->run_step($reverse_step, false, !$revert); } - // rethrow the exception throw $e; } } -- cgit v1.2.1