From 8b8f693d00ebbf78066467497b8866c751e6760f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 22 Oct 2016 19:19:36 +0200 Subject: [PATCH 1/6] [ticket/14831] Make sure migrations always start with backslash The migration system expects every migration to start with a backslash. If depends on definitions do not supply that leading backslash, we should make sure it's added manually before calling the depends on migration. PHPBB3-14831 --- phpBB/phpbb/db/migrator.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 4c4c0a8672..1f5498c878 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -264,6 +264,9 @@ class migrator foreach ($state['migration_depends_on'] as $depend) { + // Make sure migration starts with backslash + $depend = $depend[0] == '\\' ? $depend : '\\' . $depend; + if ($this->unfulfillable($depend) !== false) { throw new \phpbb\db\migration\exception('MIGRATION_NOT_FULFILLABLE', $name, $depend); From 2059d57c04ce0083079ae3f8971ff2d758bbe0c5 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 23 Oct 2016 10:28:22 +0200 Subject: [PATCH 2/6] [ticket/14831] Fall back to possible migration names instead of adding prefix Instead of just adding the backslash as prefix if needed, this will take care of falling back to any possible migration with or without backslash no matter how the mgiration was saved in the database or called in the migrations file. This will result in a more robust migrator in regards to naming the migrations and previously run migrations. PHPBB3-14831 --- phpBB/phpbb/db/migrator.php | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 1f5498c878..adfbdc43db 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -209,6 +209,19 @@ class migrator { foreach ($this->migrations as $name) { + // Try falling back to a valid migration name with or without leading backslash + if (!isset($this->migration_state[$name])) + { + if (isset($this->migration_state[preg_replace('#^(?!\\\)#', '\\\$0', $name)])) + { + $name = preg_replace('#^(?!\\\)#', '\\\$0', $name); + } + else if (isset($this->migration_state[preg_replace('#(^\\\)([^\\\].+)#', '$2', $name)])) + { + $name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); + } + } + if (!isset($this->migration_state[$name]) || !$this->migration_state[$name]['migration_schema_done'] || !$this->migration_state[$name]['migration_data_done']) @@ -264,10 +277,22 @@ class migrator foreach ($state['migration_depends_on'] as $depend) { - // Make sure migration starts with backslash - $depend = $depend[0] == '\\' ? $depend : '\\' . $depend; + // Try falling back to a valid migration name with or without leading backslash + if (!isset($this->migration_state[$name])) + { + if (isset($this->migration_state[preg_replace('#^(?!\\\)#', '\\\$0', $name)])) + { + $name = preg_replace('#^(?!\\\)#', '\\\$0', $name); + } + else if (isset($this->migration_state[preg_replace('#(^\\\)([^\\\].+)#', '$2', $name)])) + { + $name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); + } + } - if ($this->unfulfillable($depend) !== false) + // Test all possible namings before throwing exception + if ($this->unfulfillable($depend) !== false && $this->unfulfillable(preg_replace('#(^\\\)([^\\\].+)#', '$2', $depend)) !== false && + $this->unfulfillable(preg_replace('#^(?!\\\)#', '\\\$0', $name))) { throw new \phpbb\db\migration\exception('MIGRATION_NOT_FULFILLABLE', $name, $depend); } From 9f2867b115ad6e07cf7be6b2ca98ed6ef5be07c7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 23 Oct 2016 11:37:10 +0200 Subject: [PATCH 3/6] [ticket/14831] Add method for getting valid migration name PHPBB3-14831 --- phpBB/phpbb/db/migrator.php | 60 +++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index adfbdc43db..d84635f33d 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -200,6 +200,34 @@ class migrator $this->container->get('dispatcher')->enable(); } + /** + * Get a valid migration name from the migration state array in case the + * supplied name is not in the migration state list. + * + * @param string $name Migration name + * @return string Migration name + */ + protected function get_valid_name($name) + { + // Try falling back to a valid migration name with or without leading backslash + if (!isset($this->migration_state[$name])) + { + $appended_name = preg_replace('#^(?!\\\)#', '\\\$0', $name); + $prefixless_name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); + + if (isset($this->migration_state[$appended_name])) + { + $name = $appended_name; + } + else if (isset($this->migration_state[$prefixless_name])) + { + $name = $prefixless_name; + } + } + + return $name; + } + /** * Effectively runs a single update step from the next migration to be applied. * @@ -209,18 +237,7 @@ class migrator { foreach ($this->migrations as $name) { - // Try falling back to a valid migration name with or without leading backslash - if (!isset($this->migration_state[$name])) - { - if (isset($this->migration_state[preg_replace('#^(?!\\\)#', '\\\$0', $name)])) - { - $name = preg_replace('#^(?!\\\)#', '\\\$0', $name); - } - else if (isset($this->migration_state[preg_replace('#(^\\\)([^\\\].+)#', '$2', $name)])) - { - $name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); - } - } + $name = $this->get_valid_name($name); if (!isset($this->migration_state[$name]) || !$this->migration_state[$name]['migration_schema_done'] || @@ -277,22 +294,10 @@ class migrator foreach ($state['migration_depends_on'] as $depend) { - // Try falling back to a valid migration name with or without leading backslash - if (!isset($this->migration_state[$name])) - { - if (isset($this->migration_state[preg_replace('#^(?!\\\)#', '\\\$0', $name)])) - { - $name = preg_replace('#^(?!\\\)#', '\\\$0', $name); - } - else if (isset($this->migration_state[preg_replace('#(^\\\)([^\\\].+)#', '$2', $name)])) - { - $name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); - } - } + $depend = $this->get_valid_name($depend); // Test all possible namings before throwing exception - if ($this->unfulfillable($depend) !== false && $this->unfulfillable(preg_replace('#(^\\\)([^\\\].+)#', '$2', $depend)) !== false && - $this->unfulfillable(preg_replace('#^(?!\\\)#', '\\\$0', $name))) + if ($this->unfulfillable($depend) !== false) { throw new \phpbb\db\migration\exception('MIGRATION_NOT_FULFILLABLE', $name, $depend); } @@ -770,6 +775,8 @@ class migrator */ public function unfulfillable($name) { + $name = $this->get_valid_name($name); + if (isset($this->migration_state[$name]) || isset($this->fulfillable_migrations[$name])) { return false; @@ -785,6 +792,7 @@ class migrator foreach ($depends as $depend) { + $depend = $this->get_valid_name($depend); $unfulfillable = $this->unfulfillable($depend); if ($unfulfillable !== false) { From c891277996872920f88ea7bb36b1e57e3674579f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 23 Oct 2016 22:00:02 +0200 Subject: [PATCH 4/6] [ticket/14831] Add migration for deduplicating entries and fix typo PHPBB3-14831 --- .../v31x/migrations_deduplicate_entries.php | 78 +++++++++++++++++++ phpBB/phpbb/db/migrator.php | 6 +- 2 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php diff --git a/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php b/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php new file mode 100644 index 0000000000..4e539cf36d --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php @@ -0,0 +1,78 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\db\migration\data\v31x; + +class migrations_deduplicate_entries extends \phpbb\db\migration\migration +{ + static public function depends_on() + { + return array('\phpbb\db\migration\data\v31x\v3110'); + } + + public function update_data() + { + return array( + array('custom', array(array($this, 'deduplicate_entries'))), + ); + } + + public function deduplicate_entries() + { + $migration_state = array(); + $duplicate_migrations = array(); + + $sql = "SELECT * + FROM " . $this->table_prefix . 'migrations'; + $result = $this->db->sql_query($sql); + + if (!$this->db->get_sql_error_triggered()) + { + while ($migration = $this->db->sql_fetchrow($result)) + { + $migration_state[$migration['migration_name']] = $migration; + + $migration_state[$migration['migration_name']]['migration_depends_on'] = unserialize($migration['migration_depends_on']); + $migration_state[$migration['migration_name']]['migration_data_state'] = !empty($migration['migration_data_state']) ? unserialize($migration['migration_data_state']) : ''; + } + } + + $this->db->sql_freeresult($result); + + foreach ($migration_state as $name => $migration) + { + $prepended_name = preg_replace('#^(?!\\\)#', '\\\$0', $name); + $prefixless_name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); + + if ($prepended_name !== $name && isset($migration_state[$prepended_name]) && $migration_state[$prepended_name] === $migration_state[$name]) + { + $duplicate_migrations[] = $name; + unset($migration_state[$prepended_name]); + } + else if ($prefixless_name !== $name && isset($migration_state[$prefixless_name]) && $migration_state[$prefixless_name] === $migration_state[$name]) + { + $duplicate_migrations[] = $name; + unset($migration_state[$prefixless_name]); + } + } + + if (count($duplicate_migrations)) + { + $sql = 'DELETE * + FROM ' . $this->table_prefix . 'migrations + WHERE ' . $this->db->sql_in_set('migration_name', $duplicate_migrations); + $this->db->sql_query($sql); + } + } +} diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index d84635f33d..30eafcc470 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -212,12 +212,12 @@ class migrator // Try falling back to a valid migration name with or without leading backslash if (!isset($this->migration_state[$name])) { - $appended_name = preg_replace('#^(?!\\\)#', '\\\$0', $name); + $prepended_name = preg_replace('#^(?!\\\)#', '\\\$0', $name); $prefixless_name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); - if (isset($this->migration_state[$appended_name])) + if (isset($this->migration_state[$prepended_name])) { - $name = $appended_name; + $name = $prepended_name; } else if (isset($this->migration_state[$prefixless_name])) { From 6f8c0df1c68a2f7812c10ffd489098f50401022d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 23 Oct 2016 22:17:19 +0200 Subject: [PATCH 5/6] [ticket/14831] Compare depends_on for migrations and remove prefixless names PHPBB3-14831 --- .../data/v31x/migrations_deduplicate_entries.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php b/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php index 4e539cf36d..5f883952b4 100644 --- a/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php +++ b/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php @@ -55,21 +55,21 @@ class migrations_deduplicate_entries extends \phpbb\db\migration\migration $prepended_name = preg_replace('#^(?!\\\)#', '\\\$0', $name); $prefixless_name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); - if ($prepended_name !== $name && isset($migration_state[$prepended_name]) && $migration_state[$prepended_name] === $migration_state[$name]) + if ($prepended_name != $name && isset($migration_state[$prepended_name]) && $migration_state[$prepended_name]['migration_depends_on'] == $migration_state[$name]['migration_depends_on']) { $duplicate_migrations[] = $name; unset($migration_state[$prepended_name]); } - else if ($prefixless_name !== $name && isset($migration_state[$prefixless_name]) && $migration_state[$prefixless_name] === $migration_state[$name]) + else if ($prefixless_name != $name && isset($migration_state[$prefixless_name]) && $migration_state[$prefixless_name]['migration_depends_on'] == $migration_state[$name]['migration_depends_on']) { - $duplicate_migrations[] = $name; + $duplicate_migrations[] = $prefixless_name; unset($migration_state[$prefixless_name]); } } if (count($duplicate_migrations)) { - $sql = 'DELETE * + $sql = 'DELETE FROM ' . $this->table_prefix . 'migrations WHERE ' . $this->db->sql_in_set('migration_name', $duplicate_migrations); $this->db->sql_query($sql); From ffc6623dd4b66f039698d86701c49724bc217bc7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 25 Oct 2016 20:25:57 +0200 Subject: [PATCH 6/6] [ticket/14831] Rename migration and replace preg_replace() with simpler methods PHPBB3-14831 --- ...plicate_entries.php => remove_duplicate_migrations.php} | 7 +++---- phpBB/phpbb/db/migrator.php | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) rename phpBB/phpbb/db/migration/data/v31x/{migrations_deduplicate_entries.php => remove_duplicate_migrations.php} (83%) diff --git a/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php b/phpBB/phpbb/db/migration/data/v31x/remove_duplicate_migrations.php similarity index 83% rename from phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php rename to phpBB/phpbb/db/migration/data/v31x/remove_duplicate_migrations.php index 5f883952b4..417d569a09 100644 --- a/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php +++ b/phpBB/phpbb/db/migration/data/v31x/remove_duplicate_migrations.php @@ -14,7 +14,7 @@ namespace phpbb\db\migration\data\v31x; -class migrations_deduplicate_entries extends \phpbb\db\migration\migration +class remove_duplicate_migrations extends \phpbb\db\migration\migration { static public function depends_on() { @@ -44,7 +44,6 @@ class migrations_deduplicate_entries extends \phpbb\db\migration\migration $migration_state[$migration['migration_name']] = $migration; $migration_state[$migration['migration_name']]['migration_depends_on'] = unserialize($migration['migration_depends_on']); - $migration_state[$migration['migration_name']]['migration_data_state'] = !empty($migration['migration_data_state']) ? unserialize($migration['migration_data_state']) : ''; } } @@ -52,8 +51,8 @@ class migrations_deduplicate_entries extends \phpbb\db\migration\migration foreach ($migration_state as $name => $migration) { - $prepended_name = preg_replace('#^(?!\\\)#', '\\\$0', $name); - $prefixless_name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); + $prepended_name = ($name[0] == '\\' ? '' : '\\') . $name; + $prefixless_name = $name[0] == '\\' ? substr($name, 1) : $name; if ($prepended_name != $name && isset($migration_state[$prepended_name]) && $migration_state[$prepended_name]['migration_depends_on'] == $migration_state[$name]['migration_depends_on']) { diff --git a/phpBB/phpbb/db/migrator.php b/phpBB/phpbb/db/migrator.php index 30eafcc470..45a333ac94 100644 --- a/phpBB/phpbb/db/migrator.php +++ b/phpBB/phpbb/db/migrator.php @@ -212,8 +212,8 @@ class migrator // Try falling back to a valid migration name with or without leading backslash if (!isset($this->migration_state[$name])) { - $prepended_name = preg_replace('#^(?!\\\)#', '\\\$0', $name); - $prefixless_name = preg_replace('#(^\\\)([^\\\].+)#', '$2', $name); + $prepended_name = ($name[0] == '\\' ? '' : '\\') . $name; + $prefixless_name = $name[0] == '\\' ? substr($name, 1) : $name; if (isset($this->migration_state[$prepended_name])) {