From 6078bae7f8888f6c1d8bdc9a409b3e0a9cb3cc49 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 13:31:23 +0200 Subject: [PATCH 1/4] [ticket/14742] Fix schema update First make it work, then avoid a loop PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 8637f71414..aaa7769cee 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -470,6 +470,9 @@ class migrator $steps = array_reverse($steps); } + end($steps); + $last_step_identifier = key($steps); + foreach ($steps as $step_identifier => $step) { $last_result = 0; @@ -486,6 +489,12 @@ class migrator // Set state to false since we reached the point we were at $state = false; + + // There is a programmed tendency to get stuck in this case + if (strpos($step[0], 'dbtools') === 0 && ($last_result === null || $last_result === true)) + { + continue; + } } try @@ -494,7 +503,8 @@ class migrator // 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) + if (($result !== null && $result !== true) || + (strpos($step[0], 'dbtools') === 0 && $step_identifier !== $last_step_identifier)) { return serialize(array( 'result' => $result, From b00a39b9adeca86f2c2777a74d6f4f8325ee7743 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 15:37:53 +0200 Subject: [PATCH 2/4] [ticket/14742] Make $last_run_migration protected PHPBB3-14742 --- phpBB/install/database_update.php | 7 ------- phpBB/phpbb/db/migrator.php | 15 ++++++++++++++- 2 files changed, 14 insertions(+), 8 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/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index aaa7769cee..965d117b0b 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -80,7 +80,7 @@ class migrator * * @var array */ - public $last_run_migration = false; + protected $last_run_migration = false; /** * The output handler. A null handler is configured by default. @@ -160,6 +160,19 @@ class migrator $this->db->sql_return_on_error(false); } + /** + * 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. * From 52afa74f4e6080952a72b6f40f15e6ce0a547b49 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 16:38:56 +0200 Subject: [PATCH 3/4] [ticket/14742] Avoid loop while reverting schema PHPBB3-14742 --- phpBB/phpbb/db/migrator.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 965d117b0b..b82b1b918a 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -457,8 +457,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; @@ -503,8 +508,8 @@ class migrator // Set state to false since we reached the point we were at $state = false; - // There is a programmed tendency to get stuck in this case - if (strpos($step[0], 'dbtools') === 0 && ($last_result === null || $last_result === true)) + // There is a tendency to get stuck in some cases + if ($last_result === null || $last_result === true) { continue; } From c12d67cd900514f9752d7bb73928870dbab0a0ce Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Thu, 11 Aug 2016 18:11:39 +0200 Subject: [PATCH 4/4] [ticket/14742] Add test for (not) reverting if PHPBB3-14742 --- tests/dbal/migration/if.php | 4 ++-- tests/dbal/migrator_test.php | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) 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()