From 8b8f693d00ebbf78066467497b8866c751e6760f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 22 Oct 2016 19:19:36 +0200 Subject: [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(+) (limited to 'phpBB/phpbb') 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); -- cgit v1.2.1 From 2059d57c04ce0083079ae3f8971ff2d758bbe0c5 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 23 Oct 2016 10:28:22 +0200 Subject: [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(-) (limited to 'phpBB/phpbb') 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); } -- cgit v1.2.1 From 9f2867b115ad6e07cf7be6b2ca98ed6ef5be07c7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 23 Oct 2016 11:37:10 +0200 Subject: [ticket/14831] Add method for getting valid migration name PHPBB3-14831 --- phpBB/phpbb/db/migrator.php | 60 +++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 26 deletions(-) (limited to 'phpBB/phpbb') 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) { -- cgit v1.2.1 From c891277996872920f88ea7bb36b1e57e3674579f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 23 Oct 2016 22:00:02 +0200 Subject: [ticket/14831] Add migration for deduplicating entries and fix typo PHPBB3-14831 --- .../data/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 (limited to 'phpBB/phpbb') 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])) { -- cgit v1.2.1 From 6f8c0df1c68a2f7812c10ffd489098f50401022d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 23 Oct 2016 22:17:19 +0200 Subject: [ticket/14831] Compare depends_on for migrations and remove prefixless names PHPBB3-14831 --- .../db/migration/data/v31x/migrations_deduplicate_entries.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'phpBB/phpbb') 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); -- cgit v1.2.1 From ffc6623dd4b66f039698d86701c49724bc217bc7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 25 Oct 2016 20:25:57 +0200 Subject: [ticket/14831] Rename migration and replace preg_replace() with simpler methods PHPBB3-14831 --- .../data/v31x/migrations_deduplicate_entries.php | 78 ---------------------- .../data/v31x/remove_duplicate_migrations.php | 77 +++++++++++++++++++++ phpBB/phpbb/db/migrator.php | 4 +- 3 files changed, 79 insertions(+), 80 deletions(-) delete mode 100644 phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php create mode 100644 phpBB/phpbb/db/migration/data/v31x/remove_duplicate_migrations.php (limited to 'phpBB/phpbb') diff --git a/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php b/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php deleted file mode 100644 index 5f883952b4..0000000000 --- a/phpBB/phpbb/db/migration/data/v31x/migrations_deduplicate_entries.php +++ /dev/null @@ -1,78 +0,0 @@ - - * @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_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_depends_on'] == $migration_state[$name]['migration_depends_on']) - { - $duplicate_migrations[] = $prefixless_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/migration/data/v31x/remove_duplicate_migrations.php b/phpBB/phpbb/db/migration/data/v31x/remove_duplicate_migrations.php new file mode 100644 index 0000000000..417d569a09 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v31x/remove_duplicate_migrations.php @@ -0,0 +1,77 @@ + + * @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 remove_duplicate_migrations 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']); + } + } + + $this->db->sql_freeresult($result); + + foreach ($migration_state as $name => $migration) + { + $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']) + { + $duplicate_migrations[] = $name; + unset($migration_state[$prepended_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[] = $prefixless_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 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])) { -- cgit v1.2.1