diff --git a/phpBB/phpbb/db/migration/tool/permission.php b/phpBB/phpbb/db/migration/tool/permission.php index 4b53aa32a7..6d19ad94c9 100644 --- a/phpBB/phpbb/db/migration/tool/permission.php +++ b/phpBB/phpbb/db/migration/tool/permission.php @@ -240,6 +240,25 @@ class permission implements \phpbb\db\migration\tool\tool_interface $this->auth->acl_clear_prefetch(); } + /** + * Check if a permission role exists + * + * @param string $role_name The role name + * + * @return int The id of the role if it exists, 0 otherwise + */ + public function role_exists($role_name) + { + $sql = 'SELECT role_id + FROM ' . ACL_ROLES_TABLE . " + WHERE role_name = '" . $this->db->sql_escape($role_name) . "'"; + $result = $this->db->sql_query($sql); + $role_id = (int) $this->db->sql_fetchfield('role_id'); + $this->db->sql_freeresult($result); + + return $role_id; + } + /** * Add a new permission role * @@ -251,13 +270,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface */ public function role_add($role_name, $role_type, $role_description = '') { - $sql = 'SELECT role_id - FROM ' . ACL_ROLES_TABLE . " - WHERE role_name = '" . $this->db->sql_escape($role_name) . "'"; - $this->db->sql_query($sql); - $role_id = (int) $this->db->sql_fetchfield('role_id'); - - if ($role_id) + if ($this->role_exists($role_name)) { return; } @@ -290,13 +303,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface */ public function role_update($old_role_name, $new_role_name) { - $sql = 'SELECT role_id - FROM ' . ACL_ROLES_TABLE . " - WHERE role_name = '" . $this->db->sql_escape($old_role_name) . "'"; - $this->db->sql_query($sql); - $role_id = (int) $this->db->sql_fetchfield('role_id'); - - if (!$role_id) + if (!$this->role_exists($old_role_name)) { throw new \phpbb\db\migration\exception('ROLE_NOT_EXIST', $old_role_name); } @@ -315,13 +322,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface */ public function role_remove($role_name) { - $sql = 'SELECT role_id - FROM ' . ACL_ROLES_TABLE . " - WHERE role_name = '" . $this->db->sql_escape($role_name) . "'"; - $this->db->sql_query($sql); - $role_id = (int) $this->db->sql_fetchfield('role_id'); - - if (!$role_id) + if (!($role_id = $this->role_exists($role_name))) { return; } @@ -381,13 +382,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface switch ($type) { case 'role': - $sql = 'SELECT role_id - FROM ' . ACL_ROLES_TABLE . " - WHERE role_name = '" . $this->db->sql_escape($name) . "'"; - $this->db->sql_query($sql); - $role_id = (int) $this->db->sql_fetchfield('role_id'); - - if (!$role_id) + if (!($role_id = $this->role_exists($name))) { throw new \phpbb\db\migration\exception('ROLE_NOT_EXIST', $name); } @@ -539,13 +534,7 @@ class permission implements \phpbb\db\migration\tool\tool_interface switch ($type) { case 'role': - $sql = 'SELECT role_id - FROM ' . ACL_ROLES_TABLE . " - WHERE role_name = '" . $this->db->sql_escape($name) . "'"; - $this->db->sql_query($sql); - $role_id = (int) $this->db->sql_fetchfield('role_id'); - - if (!$role_id) + if (!($role_id = $this->role_exists($name))) { throw new \phpbb\db\migration\exception('ROLE_NOT_EXIST', $name); } diff --git a/tests/dbal/migrator_tool_permission_test.php b/tests/dbal/migrator_tool_permission_test.php index a48c367896..e9cf55a0ce 100644 --- a/tests/dbal/migrator_tool_permission_test.php +++ b/tests/dbal/migrator_tool_permission_test.php @@ -13,6 +13,12 @@ class phpbb_dbal_migrator_tool_permission_test extends phpbb_database_test_case { + /** @var \phpbb\auth\auth */ + protected $auth; + + /** @var \phpbb\db\migration\tool\permission */ + protected $tool; + public $group_ids = array( 'REGISTERED' => 2, 'GLOBAL_MODERATORS' => 4, @@ -218,4 +224,22 @@ class phpbb_dbal_migrator_tool_permission_test extends phpbb_database_test_case break; } } + + public function data_test_permission_role_exists() + { + return array( + array('ROLE_MOD_FULL', true), + array('ROLE_USER_FULL', true), + array('ROLE_ADMIN_STANDARD', true), + array('ROLE_DOES_NOT_EXIST', false), + ); + } + + /** + * @dataProvider data_test_permission_role_exists + */ + public function test_permission_role_exists($role_name, $expected) + { + $this->assertEquals($expected, $this->tool->role_exists($role_name)); + } }