aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--phpBB/install/database_update.php7
-rw-r--r--phpBB/language/en/migrator.php1
-rw-r--r--phpBB/phpbb/db/migration/helper.php32
-rw-r--r--phpBB/phpbb/db/migration/tool/config.php5
-rw-r--r--phpBB/phpbb/db/migration/tool/config_text.php5
-rw-r--r--phpBB/phpbb/db/migration/tool/module.php5
-rw-r--r--phpBB/phpbb/db/migration/tool/permission.php5
-rw-r--r--phpBB/phpbb/db/migrator.php107
-rw-r--r--tests/dbal/migration/if.php4
-rw-r--r--tests/dbal/migrator_test.php8
-rw-r--r--tests/migrator/reverse_update_data_test.php56
11 files changed, 201 insertions, 34 deletions
diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php
index 0ea6eeffd7..f367ae1fc0 100644
--- a/phpBB/install/database_update.php
+++ b/phpBB/install/database_update.php
@@ -211,13 +211,6 @@ while (!$migrator->finished())
phpbb_end_update($cache, $config);
}
- $state = array_merge(array(
- 'migration_schema_done' => false,
- 'migration_data_done' => false,
- ),
- $migrator->last_run_migration['state']
- );
-
// Are we approaching the time limit? If so we want to pause the update and continue after refreshing
if ((time() - $update_start_time) >= $safe_time_limit)
{
diff --git a/phpBB/language/en/migrator.php b/phpBB/language/en/migrator.php
index 244a5faadf..7623636941 100644
--- a/phpBB/language/en/migrator.php
+++ b/phpBB/language/en/migrator.php
@@ -50,6 +50,7 @@ $lang = array_merge($lang, array(
'MIGRATION_NOT_FULFILLABLE' => 'The migration "%1$s" is not fulfillable, missing migration "%2$s".',
'MIGRATION_NOT_VALID' => '%s is not a valid migration.',
'MIGRATION_SCHEMA_DONE' => 'Installed Schema: %1$s; Time: %2$.2f seconds',
+ 'MIGRATION_SCHEMA_IN_PROGRESS' => 'Installing Schema: %1$s; Time: %2$.2f seconds',
'MIGRATION_SCHEMA_RUNNING' => 'Installing Schema: %s.',
'MIGRATION_INVALID_DATA_MISSING_CONDITION' => 'A migration is invalid. An if statement helper is missing a condition.',
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 7fc3e787e2..65db4b74bf 100644
--- a/phpBB/phpbb/db/migrator.php
+++ b/phpBB/phpbb/db/migrator.php
@@ -80,14 +80,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
@@ -152,6 +152,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']) : '';
}
}
@@ -161,6 +162,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
@@ -297,38 +311,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
{
@@ -340,7 +382,6 @@ class migrator
// Revert the schema changes
$this->revert_do($name);
- // Rethrow exception
throw $e;
}
}
@@ -416,19 +457,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);
}
@@ -446,8 +479,13 @@ 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]);
}
+ else
+ {
+ $this->set_migration_state($name, $state);
+ }
}
return true;
@@ -464,7 +502,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)
@@ -472,6 +510,9 @@ class migrator
$steps = array_reverse($steps);
}
+ end($steps);
+ $last_step_identifier = key($steps);
+
foreach ($steps as $step_identifier => $step)
{
$last_result = 0;
@@ -488,18 +529,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)
@@ -517,7 +567,6 @@ class migrator
$result = $this->run_step($reverse_step, false, !$revert);
}
- // rethrow the exception
throw $e;
}
}
@@ -587,6 +636,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)
@@ -664,6 +720,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/tests/dbal/migration/if.php b/tests/dbal/migration/if.php
index 98a66526ed..481250ea77 100644
--- a/tests/dbal/migration/if.php
+++ b/tests/dbal/migration/if.php
@@ -36,13 +36,13 @@ class phpbb_dbal_migration_if extends \phpbb\db\migration\migration
{
global $migrator_test_if_true_failed;
- $migrator_test_if_true_failed = false;
+ $migrator_test_if_true_failed = !$migrator_test_if_true_failed;
}
function test_false()
{
global $migrator_test_if_false_failed;
- $migrator_test_if_false_failed = true;
+ $migrator_test_if_false_failed = !$migrator_test_if_false_failed;
}
}
diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php
index 4c4306888c..798200eef1 100644
--- a/tests/dbal/migrator_test.php
+++ b/tests/dbal/migrator_test.php
@@ -156,6 +156,14 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
$this->assertFalse($migrator_test_if_true_failed, 'True test failed');
$this->assertFalse($migrator_test_if_false_failed, 'False test failed');
+
+ while ($this->migrator->migration_state('phpbb_dbal_migration_if') !== false)
+ {
+ $this->migrator->revert('phpbb_dbal_migration_if');
+ }
+
+ $this->assertFalse($migrator_test_if_true_failed, 'True test after revert failed');
+ $this->assertFalse($migrator_test_if_false_failed, 'False test after revert failed');
}
public function test_recall()
diff --git a/tests/migrator/reverse_update_data_test.php b/tests/migrator/reverse_update_data_test.php
new file mode 100644
index 0000000000..b85e48c64c
--- /dev/null
+++ b/tests/migrator/reverse_update_data_test.php
@@ -0,0 +1,56 @@
+<?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.
+ *
+ */
+
+class reverse_update_data_test extends phpbb_test_case
+{
+ /** @var \phpbb\db\migration\helper */
+ protected $helper;
+
+ public function setUp()
+ {
+ parent::setUp();
+
+ $this->helper = new \phpbb\db\migration\helper();
+ }
+
+ public function update_data_provider()
+ {
+ return array(
+ array(
+ array(
+ array('config.add', array('foobar', 1)),
+ array('if', array(
+ (false === true),
+ array('permission.add', array('some_data')),
+ )),
+ array('config.remove', array('foobar')),
+ array('custom', array(array($this, 'foo_bar'))),
+ array('tool.method', array('test_data')),
+ ),
+ array(
+ array('tool.reverse', array('method', 'test_data')),
+ array('config.reverse', array('remove', 'foobar')),
+ array('config.reverse', array('add', 'foobar', 1)),
+ ),
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider update_data_provider
+ */
+ public function test_get_schema_steps($data_changes, $expected)
+ {
+ $this->assertEquals($expected, $this->helper->reverse_update_data($data_changes));
+ }
+}