Merge pull request #4419 from Elsensee/ticket/14742

[ticket/14742] Further improvements to migrator

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

@@ -152,7 +152,7 @@ class config implements \phpbb\db\migration\tool\tool_interface
 			break;
 
 			case 'reverse':
-				// It's like double negative
+				// Reversing a reverse is just the call itself
 				$call = array_shift($arguments);
 			break;
 		}

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

@@ -117,7 +117,7 @@ class config_text implements \phpbb\db\migration\tool\tool_interface
 			break;
 
 			case 'reverse':
-				// It's like double negative
+				// Reversing a reverse is just the call itself
 				$call = array_shift($arguments);
 			break;
 		}

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

@@ -456,7 +456,7 @@ class module implements \phpbb\db\migration\tool\tool_interface
 			break;
 
 			case 'reverse':
-				// It's like double negative
+				// Reversing a reverse is just the call itself
 				$call = array_shift($arguments);
 			break;
 		}

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

@@ -639,7 +639,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface
 			break;
 
 			case 'reverse':
-				// It's like double negative
+				// Reversing a reverse is just the call itself
 				$call = array_shift($arguments);
 			break;
 		}

phpBB/phpbb/db/migrator.php

@@ -320,8 +320,10 @@ class migrator
 			$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;
 
@@ -355,7 +357,9 @@ class migrator
 				$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;
 
@@ -379,7 +383,10 @@ class migrator
 			}
 			catch (\phpbb\db\migration\exception $e)
 			{
-				// Revert the schema changes
+				// Reset data state and revert the schema changes
+				$state['migration_data_state'] = '';
+				$this->set_migration_state($name, $state);
+
 				$this->revert_do($name);
 
 				throw $e;
@@ -502,6 +509,11 @@ class migrator
 	*/
 	protected function process_data_step($steps, $state, $revert = false)
 	{
+		if (sizeof($steps) === 0)
+		{
+			return true;
+		}
+
 		$state = is_array($state) ? $state : false;
 
 		// reverse order of steps if reverting
@@ -510,65 +522,45 @@ class migrator
 			$steps = array_reverse($steps);
 		}
 
-		end($steps);
+		$step = $last_result = 0;
-		$last_step_identifier = key($steps);
+		if ($state)
-
-		foreach ($steps as $step_identifier => $step)
 		{
-			$last_result = 0;
+			$step = $state['step'];
-			if ($state)
+
+			// We send the result from last time to the callable function
+			$last_result = $state['result'];
+		}
+
+		try
+		{
+			// Result will be null or true if everything completed correctly
+			// Stop after each update step, to let the updater control the script runtime
+			$result = $this->run_step($steps[$step], $last_result, $revert);
+			if (($result !== null && $result !== true) || $step + 1 < sizeof($steps))
 			{
-				// Continue until we reach the step that matches the last step called
+				return array(
-				if ($state['step'] != $step_identifier)
+					'result'	=> $result,
+					// Move on if the last call finished
+					'step'		=> ($result !== null && $result !== true) ? $step : $step + 1,
+				);
+			}
+		}
+		catch (\phpbb\db\migration\exception $e)
+		{
+			// We should try rolling back here
+			foreach ($steps as $reverse_step_identifier => $reverse_step)
+			{
+				// If we've reached the current step we can break because we reversed everything that was run
+				if ($reverse_step_identifier == $step)
 				{
-					continue;
+					break;
 				}
 
-				// We send the result from last time to the callable function
+				// Reverse the step that was run
-				$last_result = $state['result'];
+				$result = $this->run_step($reverse_step, false, !$revert);
-
-				// 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
+			throw $e;
-			{
-				// 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) ||
-					(strpos($step[0], 'dbtools') === 0 && $step_identifier !== $last_step_identifier))
-				{
-					return array(
-						'result'	=> $result,
-						'step'		=> $step_identifier,
-					);
-				}
-			}
-			catch (\phpbb\db\migration\exception $e)
-			{
-				// We should try rolling back here
-				foreach ($steps as $reverse_step_identifier => $reverse_step)
-				{
-					// If we've reached the current step we can break because we reversed everything that was run
-					if ($reverse_step_identifier == $step_identifier)
-					{
-						break;
-					}
-
-					// Reverse the step that was run
-					$result = $this->run_step($reverse_step, false, !$revert);
-				}
-
-				throw $e;
-			}
 		}
 
 		return true;

phpBB/phpbb/db/migrator_output_handler_interface.php

@@ -15,11 +15,11 @@ namespace phpbb\db;
 
 interface migrator_output_handler_interface
 {
-	const VERBOSITY_QUIET        = 0;
+	const VERBOSITY_QUIET        = 16;
-	const VERBOSITY_NORMAL       = 1;
+	const VERBOSITY_NORMAL       = 32;
-	const VERBOSITY_VERBOSE      = 2;
+	const VERBOSITY_VERBOSE      = 64;
-	const VERBOSITY_VERY_VERBOSE = 3;
+	const VERBOSITY_VERY_VERBOSE = 128;
-	const VERBOSITY_DEBUG        = 4;
+	const VERBOSITY_DEBUG        = 256;
 
 	/**
 	 * Write output using the configured closure.