Merge pull request #4410 from Elsensee/ticket/14742-32x

[ticket/14742-32x] Improvements to migrator
author: Marc Alexander <admin@m-a-styles.de> 2016-08-13 23:13:07 +0200
committer: Marc Alexander <admin@m-a-styles.de> 2016-08-13 23:13:07 +0200
commit: 4466ef1bc4b66a4396054aa5e3c4eec9c97f231b (patch)
tree: dce8ad8a5dc05069ee1df7b5c08987129361d18e /phpBB/phpbb
parent: 55f98d0941e17b473015baf07026efabaf676916 (diff)
parent: 758fe20f4be7178fd4b9fd6ce48c5342cfcbce27 (diff)
download: forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.tar
forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.tar.gz
forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.tar.bz2
forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.tar.xz
forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.zip
7 files changed, 207 insertions, 35 deletions
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 a5ed62fd65..bf992a6ff1 100644
--- a/phpBB/phpbb/db/migration/tool/module.php
+++ b/phpBB/phpbb/db/migration/tool/module.php
@@ -443,6 +443,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 a1e93942cd..ff3b15df5a 100644
--- a/phpBB/phpbb/db/migrator.php
+++ b/phpBB/phpbb/db/migrator.php
@@ -82,14 +82,14 @@ class migrator
 	*
 	* @var array
 	*/
-	public $last_run_migration = false;
+	protected $last_run_migration = false;
 
 	/**
 	 * The output handler. A null handler is configured by default.
 	 *
 	 * @var migrator_output_handler_interface
 	 */
-	public $output_handler;
+	protected $output_handler;
 
 	/**
 	* Constructor of the database migrator
@@ -154,6 +154,7 @@ class migrator
 				$this->migration_state[$migration['migration_name']] = $migration;
 
 				$this->migration_state[$migration['migration_name']]['migration_depends_on'] = unserialize($migration['migration_depends_on']);
+				$this->migration_state[$migration['migration_name']]['migration_data_state'] = !empty($migration['migration_data_state']) ? unserialize($migration['migration_data_state']) : '';
 			}
 		}
 
@@ -163,6 +164,19 @@ class migrator
 	}
 
 	/**
+	 * Get an array with information about the last migration run.
+	 *
+	 * The array contains 'name', 'class' and 'state'. 'effectively_installed' is set
+	 * and set to true if the last migration was effectively_installed.
+	 *
+	 * @return array
+	 */
+	public function get_last_run_migration()
+	{
+		return $this->last_run_migration;
+	}
+
+	/**
 	* Sets the list of available migration class names to the given array.
 	*
 	* @param array $class_names An array of migration class names
@@ -192,6 +206,28 @@ class migrator
 	}
 
 	/**
+	 * Get the list of available and not installed migration class names
+	 *
+	 * @return array
+	 */
+	public function get_installable_migrations()
+	{
+		$unfinished_migrations = array();
+
+		foreach ($this->migrations as $name)
+		{
+			if (!isset($this->migration_state[$name]) ||
+				!$this->migration_state[$name]['migration_schema_done'] ||
+				!$this->migration_state[$name]['migration_data_done'])
+			{
+				$unfinished_migrations[] = $name;
+			}
+		}
+
+		return $unfinished_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
@@ -317,38 +353,66 @@ class migrator
 
 		if (!$state['migration_schema_done'])
 		{
-			$this->output_handler->write(array('MIGRATION_SCHEMA_RUNNING', $name), migrator_output_handler_interface::VERBOSITY_VERBOSE);
+			$verbosity = empty($state['migration_data_state']) ?
+				migrator_output_handler_interface::VERBOSITY_VERBOSE : migrator_output_handler_interface::VERBOSITY_DEBUG;
+			$this->output_handler->write(array('MIGRATION_SCHEMA_RUNNING', $name), $verbosity);
 
 			$this->last_run_migration['task'] = 'process_schema_step';
+
+			$total_time = (is_array($state['migration_data_state']) && isset($state['migration_data_state']['_total_time'])) ?
+				$state['migration_data_state']['_total_time'] : 0.0;
 			$elapsed_time = microtime(true);
 			$steps = $this->helper->get_schema_steps($migration->update_schema());
 			$result = $this->process_data_step($steps, $state['migration_data_state']);
 			$elapsed_time = microtime(true) - $elapsed_time;
+			$total_time += $elapsed_time;
+
+			if (is_array($result))
+			{
+				$result['_total_time'] = $total_time;
+			}
 
 			$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, $total_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'])
 		{
 			try
 			{
-				$this->output_handler->write(array('MIGRATION_DATA_RUNNING', $name), migrator_output_handler_interface::VERBOSITY_VERBOSE);
+				$verbosity = empty($state['migration_data_state']) ?
+					migrator_output_handler_interface::VERBOSITY_VERBOSE : migrator_output_handler_interface::VERBOSITY_DEBUG;
+				$this->output_handler->write(array('MIGRATION_DATA_RUNNING', $name), $verbosity);
 
 				$this->last_run_migration['task'] = 'process_data_step';
 
+				$total_time = (is_array($state['migration_data_state']) && isset($state['migration_data_state']['_total_time'])) ?
+					$state['migration_data_state']['_total_time'] : 0.0;
 				$elapsed_time = microtime(true);
 				$result = $this->process_data_step($migration->update_data(), $state['migration_data_state']);
 				$elapsed_time = microtime(true) - $elapsed_time;
+				$total_time += $elapsed_time;
+
+				if (is_array($result))
+				{
+					$result['_total_time'] = $total_time;
+				}
 
 				$state['migration_data_state'] = ($result === true) ? '' : $result;
 				$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);
+					$this->output_handler->write(array('MIGRATION_DATA_DONE', $name, $total_time), migrator_output_handler_interface::VERBOSITY_NORMAL);
 				}
 				else
 				{
@@ -360,7 +424,6 @@ class migrator
 				// Revert the schema changes
 				$this->revert_do($name);
 
-				// Rethrow exception
 				throw $e;
 			}
 		}
@@ -436,29 +499,31 @@ class migrator
 
 		if ($state['migration_data_done'])
 		{
-			$this->output_handler->write(array('MIGRATION_REVERT_DATA_RUNNING', $name), migrator_output_handler_interface::VERBOSITY_VERBOSE);
+			$verbosity = empty($state['migration_data_state']) ?
+				migrator_output_handler_interface::VERBOSITY_VERBOSE : migrator_output_handler_interface::VERBOSITY_DEBUG;
+			$this->output_handler->write(array('MIGRATION_REVERT_DATA_RUNNING', $name), $verbosity);
+
+			$total_time = (is_array($state['migration_data_state']) && isset($state['migration_data_state']['_total_time'])) ?
+				$state['migration_data_state']['_total_time'] : 0.0;
 			$elapsed_time = microtime(true);
+			$steps = array_merge($this->helper->reverse_update_data($migration->update_data()), $migration->revert_data());
+			$result = $this->process_data_step($steps, $state['migration_data_state']);
+			$elapsed_time = microtime(true) - $elapsed_time;
+			$total_time += $elapsed_time;
 
-			if ($state['migration_data_state'] !== 'revert_data')
+			if (is_array($result))
 			{
-				$result = $this->process_data_step($migration->update_data(), $state['migration_data_state'], true);
-
-				$state['migration_data_state'] = ($result === true) ? 'revert_data' : $result;
+				$result['_total_time'] = $total_time;
 			}
-			else
-			{
-				$result = $this->process_data_step($migration->revert_data(), '', false);
 
-				$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);
 
-			$elapsed_time = microtime(true) - $elapsed_time;
-			if ($state['migration_data_done'])
+			if (!$state['migration_data_done'])
 			{
-				$this->output_handler->write(array('MIGRATION_REVERT_DATA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL);
+				$this->output_handler->write(array('MIGRATION_REVERT_DATA_DONE', $name, $total_time), migrator_output_handler_interface::VERBOSITY_NORMAL);
 			}
 			else
 			{
@@ -467,11 +532,22 @@ class migrator
 		}
 		else if ($state['migration_schema_done'])
 		{
-			$this->output_handler->write(array('MIGRATION_REVERT_SCHEMA_RUNNING', $name), migrator_output_handler_interface::VERBOSITY_VERBOSE);
-			$elapsed_time = microtime(true);
+			$verbosity = empty($state['migration_data_state']) ?
+				migrator_output_handler_interface::VERBOSITY_VERBOSE : migrator_output_handler_interface::VERBOSITY_DEBUG;
+			$this->output_handler->write(array('MIGRATION_REVERT_SCHEMA_RUNNING', $name), $verbosity);
 
+			$total_time = (is_array($state['migration_data_state']) && isset($state['migration_data_state']['_total_time'])) ?
+				$state['migration_data_state']['_total_time'] : 0.0;
+			$elapsed_time = microtime(true);
 			$steps = $this->helper->get_schema_steps($migration->revert_schema());
 			$result = $this->process_data_step($steps, $state['migration_data_state']);
+			$elapsed_time = microtime(true) - $elapsed_time;
+			$total_time += $elapsed_time;
+
+			if (is_array($result))
+			{
+				$result['_total_time'] = $total_time;
+			}
 
 			$state['migration_data_state'] = ($result === true) ? '' : $result;
 			$state['migration_schema_done'] = ($result === true) ? false : true;
@@ -482,11 +558,17 @@ class migrator
 					WHERE migration_name = '" . $this->db->sql_escape($name) . "'";
 				$this->db->sql_query($sql);
 
+				$this->last_run_migration = false;
 				unset($this->migration_state[$name]);
+
+				$this->output_handler->write(array('MIGRATION_REVERT_SCHEMA_DONE', $name, $total_time), migrator_output_handler_interface::VERBOSITY_NORMAL);
 			}
+			else
+			{
+				$this->set_migration_state($name, $state);
 
-			$elapsed_time = microtime(true) - $elapsed_time;
-			$this->output_handler->write(array('MIGRATION_REVERT_SCHEMA_DONE', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_NORMAL);
+				$this->output_handler->write(array('MIGRATION_REVERT_SCHEMA_IN_PROGRESS', $name, $elapsed_time), migrator_output_handler_interface::VERBOSITY_VERY_VERBOSE);
+			}
 		}
 
 		return true;
@@ -503,7 +585,7 @@ class migrator
 	*/
 	protected function process_data_step($steps, $state, $revert = false)
 	{
-		$state = ($state) ? unserialize($state) : false;
+		$state = is_array($state) ? $state : false;
 
 		// reverse order of steps if reverting
 		if ($revert === true)
@@ -511,6 +593,9 @@ class migrator
 			$steps = array_reverse($steps);
 		}
 
+		end($steps);
+		$last_step_identifier = key($steps);
+
 		foreach ($steps as $step_identifier => $step)
 		{
 			$last_result = 0;
@@ -527,18 +612,27 @@ class migrator
 
 				// Set state to false since we reached the point we were at
 				$state = false;
+
+				// There is a tendency to get stuck in some cases
+				if ($last_result === null || $last_result === true)
+				{
+					continue;
+				}
 			}
 
 			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 && $step_identifier !== $last_step_identifier))
 				{
-					return serialize(array(
+					return array(
 						'result'	=> $result,
 						'step'		=> $step_identifier,
-					));
+					);
 				}
 			}
 			catch (\phpbb\db\migration\exception $e)
@@ -556,7 +650,6 @@ class migrator
 					$this->run_step($reverse_step, false, !$revert);
 				}
 
-				// rethrow the exception
 				throw $e;
 			}
 		}
@@ -626,6 +719,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)
@@ -703,6 +803,7 @@ class migrator
 	{
 		$migration_row = $state;
 		$migration_row['migration_depends_on'] = serialize($state['migration_depends_on']);
+		$migration_row['migration_data_state'] = !empty($state['migration_data_state']) ? serialize($state['migration_data_state']) : '';
 
 		if (isset($this->migration_state[$name]))
 		{
diff --git a/phpBB/phpbb/install/module/update_database/task/update.php b/phpBB/phpbb/install/module/update_database/task/update.php
index 9fed2317e9..c69dafaa10 100644
--- a/phpBB/phpbb/install/module/update_database/task/update.php
+++ b/phpBB/phpbb/install/module/update_database/task/update.php
@@ -140,17 +140,36 @@ class update extends task_base
 			->get_classes();
 
 		$this->migrator->set_migrations($migrations);
-		$migration_count = count($this->migrator->get_migrations());
-		$this->iohandler->set_task_count($migration_count, true);
-		$this->installer_config->set_task_progress_count($migration_count);
+
+		$migration_step_count = $this->installer_config->get('database_update_migration_steps', -1);
+		if ($migration_step_count < 0)
+		{
+			$migration_step_count = count($this->migrator->get_installable_migrations()) * 2;
+			$this->installer_config->set('database_update_migration_steps', $migration_step_count);
+		}
+
 		$progress_count = $this->installer_config->get('database_update_count', 0);
+		$restart_progress_bar = ($progress_count === 0); // Only "restart" when the update runs for the first time
+		$this->iohandler->set_task_count($migration_step_count, $restart_progress_bar);
+		$this->installer_config->set_task_progress_count($migration_step_count);
 
 		while (!$this->migrator->finished())
 		{
 			try
 			{
 				$this->migrator->update();
-				$progress_count++;
+
+				$last_run_migration = $this->migrator->get_last_run_migration();
+				if (isset($last_run_migration['effectively_installed']) && $last_run_migration['effectively_installed'])
+				{
+					$progress_count += 2;
+				}
+				else if (($last_run_migration['task'] === 'process_schema_step' && $last_run_migration['state']['migration_schema_done']) ||
+					($last_run_migration['task'] === 'process_data_step' && $last_run_migration['state']['migration_data_done']))
+				{
+					$progress_count++;
+				}
+
 				$this->iohandler->set_progress('STAGE_UPDATE_DATABASE', $progress_count);
 			}
 			catch (exception $e)
author	Marc Alexander <admin@m-a-styles.de>	2016-08-13 23:13:07 +0200
committer	Marc Alexander <admin@m-a-styles.de>	2016-08-13 23:13:07 +0200
commit	4466ef1bc4b66a4396054aa5e3c4eec9c97f231b (patch)
tree	dce8ad8a5dc05069ee1df7b5c08987129361d18e /phpBB/phpbb
parent	55f98d0941e17b473015baf07026efabaf676916 (diff)
parent	758fe20f4be7178fd4b9fd6ce48c5342cfcbce27 (diff)
download	forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.tar forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.tar.gz forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.tar.bz2 forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.tar.xz forums-4466ef1bc4b66a4396054aa5e3c4eec9c97f231b.zip