-
Bug
-
Resolution: Fixed
-
Major
-
4.0.0-a1, 3.3.15
-
None
There's an annoying bug with reverting migrations in common, and reverting modules removal in particular.
Migrator performs merging of reversing migration's `update_data()` result with its `revert_data()` result:
$steps = array_merge($this->helper->reverse_update_data($migration->update_data()), $migration->revert_data()); |
https://github.com/phpbb/phpbb/blob/4367bc8d11159b876100f63c8e316b66af4ec64c/phpBB/phpbb/db/migrator.php#L578
This causes all reverting instructions to run twice, so reversing will be done for config options and permission options with an empty values; on the following attempt of applying `revert_data()` adding same config/permission options will be ignored as those options already exist:
if (isset($this->config[$config_name])) |
{
|
return; |
}
|
if ($this->exists($auth_option, $global)) |
{
|
return; |
}
|
https://github.com/phpbb/phpbb/blob/4367bc8d11159b876100f63c8e316b66af4ec64c/phpBB/phpbb/db/migration/tool/permission.php#L122-L125
Thus, incorrect config/permission options values apply because of that.
When it comes to the module removal, reversing it will add a module, and the following attempt of applying `revert_data()` will throw `\phpbb\db\migration\exception('MODULE_EXISTS', $data['module_langname'])` exception because the module was already added:
if ($this->exists($class, $parent, $data['module_langname'])) |
{
|
throw new \phpbb\db\migration\exception('MODULE_EXISTS', $data['module_langname']); |
}
|
https://github.com/phpbb/phpbb/blob/4367bc8d11159b876100f63c8e316b66af4ec64c/phpBB/phpbb/db/migration/tool/module.php#L222-L225
Additionally, because of module removal was done with the instruction like:
['module.remove', [ |
'acp', |
'ACP_CLIENT_COMMUNICATION', |
'ACP_JABBER_SETTINGS', |
]],
|
reversing it will add the module back but with empty `module_basename`, `module_mode` and `module_auth`.
Reverting steps example (as a proof):
array (size=26) |
0 => |
array (size=2) |
0 => string 'permission.reverse' (length=18) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'u_sendim' (length=8) |
1 => |
array (size=2) |
0 => string 'permission.reverse' (length=18) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'a_jabber' (length=8) |
2 => |
array (size=2) |
0 => string 'module.reverse' (length=14) |
1 => |
array (size=4) |
0 => string 'remove' (length=6) |
1 => string 'acp' (length=3) |
2 => string 'ACP_CLIENT_COMMUNICATION' (length=24) |
3 => string 'ACP_JABBER_SETTINGS' (length=19) |
3 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_allow_self_signed' (length=21) |
4 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_verify_peer_name' (length=20) |
5 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_verify_peer' (length=15) |
6 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_username' (length=12) |
7 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_use_ssl' (length=11) |
8 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_port' (length=8) |
9 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_password' (length=12) |
10 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_package_size' (length=16) |
11 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_host' (length=8) |
12 => |
array (size=2) |
0 => string 'config.reverse' (length=14) |
1 => |
array (size=2) |
0 => string 'remove' (length=6) |
1 => string 'jab_enable' (length=10) |
13 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_enable' (length=10) |
1 => int 0 |
14 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_host' (length=8) |
1 => string '' (length=0) |
15 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_package_size' (length=16) |
1 => int 20 |
16 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_password' (length=12) |
1 => string '' (length=0) |
17 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_port' (length=8) |
1 => int 5222 |
18 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_use_ssl' (length=11) |
1 => int 0 |
19 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_username' (length=12) |
1 => string '' (length=0) |
20 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_verify_peer' (length=15) |
1 => int 1 |
21 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_verify_peer_name' (length=20) |
1 => int 1 |
22 => |
array (size=2) |
0 => string 'config.add' (length=10) |
1 => |
array (size=2) |
0 => string 'jab_allow_self_signed' (length=21) |
1 => int 0 |
23 => |
array (size=2) |
0 => string 'module.add' (length=10) |
1 => |
array (size=3) |
0 => string 'acp' (length=3) |
1 => string 'ACP_CLIENT_COMMUNICATION' (length=24) |
2 => |
array (size=4) |
...
|
24 => |
array (size=2) |
0 => string 'permission.add' (length=14) |
1 => |
array (size=2) |
0 => string 'a_jabber' (length=8) |
1 => boolean true |
25 => |
array (size=2) |
0 => string 'permission.add' (length=14) |
1 => |
array (size=2) |
0 => string 'u_sendim' (length=8) |
1 => boolean true |