From 2003152c8dd9c760135ec831b49e41adcfd02142 Mon Sep 17 00:00:00 2001 From: Meik Sievertsen Date: Sat, 23 Feb 2008 14:06:46 +0000 Subject: [PATCH] - Remove left join for query used to retrieve already assigned users and groups within permission panel - #20235 - also test the serialize/unserialize approach for cached roles git-svn-id: file:///svn/phpbb/branches/phpBB-3_0_0@8390 89ea8834-ac86-4346-8a33-228a782c2dd0 --- phpBB/docs/CHANGELOG.html | 2 +- phpBB/includes/acp/acp_permissions.php | 95 +++++++++++++------------- phpBB/includes/acp/auth.php | 2 +- phpBB/includes/auth.php | 18 +++-- 4 files changed, 65 insertions(+), 52 deletions(-) diff --git a/phpBB/docs/CHANGELOG.html b/phpBB/docs/CHANGELOG.html index e55708cfb5..34686de59f 100644 --- a/phpBB/docs/CHANGELOG.html +++ b/phpBB/docs/CHANGELOG.html @@ -125,7 +125,7 @@
  • [Fix] Custom BBCode {EMAIL}-Token usage (Bug #21155)
  • [Fix] Do not rely on parameter returned by unlink() for verifying cache directory write permission (Bug #19565)
  • [Change] Use correct string for filesize (MiB instead of MB for example)
    • - +
  • [Change] Remove left join for query used to retrieve already assigned users and groups within permission panel (Bug #20235)

    • 1.i. Changes since 3.0.RC8

    diff --git a/phpBB/includes/acp/acp_permissions.php b/phpBB/includes/acp/acp_permissions.php index a19a350646..a9e64b74ae 100644 --- a/phpBB/includes/acp/acp_permissions.php +++ b/phpBB/includes/acp/acp_permissions.php @@ -124,7 +124,7 @@ class acp_permissions $forum_id = array(); while ($row = $db->sql_fetchrow($result)) { - $forum_id[] = $row['forum_id']; + $forum_id[] = (int) $row['forum_id']; } $db->sql_freeresult($result); } @@ -133,7 +133,7 @@ class acp_permissions $forum_id = array(); foreach (get_forum_branch($subforum_id, 'children') as $row) { - $forum_id[] = $row['forum_id']; + $forum_id[] = (int) $row['forum_id']; } } @@ -598,7 +598,7 @@ class acp_permissions $ids = array(); while ($row = $db->sql_fetchrow($result)) { - $ids[] = $row[$sql_id]; + $ids[] = (int) $row[$sql_id]; } $db->sql_freeresult($result); } @@ -1117,31 +1117,51 @@ class acp_permissions global $db, $user; $sql_forum_id = ($permission_scope == 'global') ? 'AND a.forum_id = 0' : ((sizeof($forum_id)) ? 'AND ' . $db->sql_in_set('a.forum_id', $forum_id) : 'AND a.forum_id <> 0'); - $sql_permission_option = ' AND o.auth_option ' . $db->sql_like_expression($permission_type . $db->any_char); - - $sql = $db->sql_build_query('SELECT_DISTINCT', array( - 'SELECT' => 'u.username, u.username_clean, u.user_regdate, u.user_id', - 'FROM' => array( - USERS_TABLE => 'u', - ACL_OPTIONS_TABLE => 'o', - ACL_USERS_TABLE => 'a' - ), + // Permission options are only able to be a permission set... therefore we will pre-fetch the possible options and also the possible roles + $option_ids = $role_ids = array(); - 'LEFT_JOIN' => array( - array( - 'FROM' => array(ACL_ROLES_DATA_TABLE => 'r'), - 'ON' => 'a.auth_role_id = r.role_id' - ) - ), + $sql = 'SELECT auth_option_id + FROM ' . ACL_OPTIONS_TABLE . ' + WHERE auth_option ' . $db->sql_like_expression($permission_type . $db->any_char); + $result = $db->sql_query($sql); - 'WHERE' => "(a.auth_option_id = o.auth_option_id OR r.auth_option_id = o.auth_option_id) - $sql_permission_option + while ($row = $db->sql_fetchrow($result)) + { + $option_ids[] = (int) $row['auth_option_id']; + } + $db->sql_freeresult($result); + + if (sizeof($option_ids)) + { + $sql = 'SELECT DISTINCT role_id + FROM ' . ACL_ROLES_DATA_TABLE . ' + WHERE ' . $db->sql_in_set('auth_option_id', $option_ids); + $result = $db->sql_query($sql); + + while ($row = $db->sql_fetchrow($result)) + { + $role_ids[] = (int) $row['role_id']; + } + $db->sql_freeresult($result); + } + + if (sizeof($option_ids) && sizeof($role_ids)) + { + $sql_where = 'AND (' . $db->sql_in_set('a.auth_option_id', $option_ids) . ' OR ' . $db->sql_in_set('a.auth_role_id', $role_ids) . ')'; + } + else + { + $sql_where = 'AND ' . $db->sql_in_set('a.auth_option_id', $option_ids); + } + + // Not ideal, due to the filesort, non-use of indexes, etc. + $sql = 'SELECT DISTINCT u.user_id, u.username + FROM ' . USERS_TABLE . ' u, ' . ACL_USERS_TABLE . " a + WHERE u.user_id = a.user_id $sql_forum_id - AND u.user_id = a.user_id", - - 'ORDER_BY' => 'u.username_clean, u.user_regdate ASC' - )); + $sql_where + ORDER BY u.username_clean, u.user_regdate ASC"; $result = $db->sql_query($sql); $s_defined_user_options = ''; @@ -1153,29 +1173,12 @@ class acp_permissions } $db->sql_freeresult($result); - $sql = $db->sql_build_query('SELECT_DISTINCT', array( - 'SELECT' => 'g.group_type, g.group_name, g.group_id', - - 'FROM' => array( - GROUPS_TABLE => 'g', - ACL_OPTIONS_TABLE => 'o', - ACL_GROUPS_TABLE => 'a' - ), - - 'LEFT_JOIN' => array( - array( - 'FROM' => array(ACL_ROLES_DATA_TABLE => 'r'), - 'ON' => 'a.auth_role_id = r.role_id' - ) - ), - - 'WHERE' => "(a.auth_option_id = o.auth_option_id OR r.auth_option_id = o.auth_option_id) - $sql_permission_option + $sql = 'SELECT DISTINCT g.group_type, g.group_name, g.group_id + FROM ' . GROUPS_TABLE . ' g, ' . ACL_GROUPS_TABLE . " a + WHERE g.group_id = a.group_id $sql_forum_id - AND g.group_id = a.group_id", - - 'ORDER_BY' => 'g.group_type DESC, g.group_name ASC' - )); + $sql_where + ORDER BY g.group_type DESC, g.group_name ASC"; $result = $db->sql_query($sql); $s_defined_group_options = ''; diff --git a/phpBB/includes/acp/auth.php b/phpBB/includes/acp/auth.php index 71872ceb6a..6943f5ada1 100644 --- a/phpBB/includes/acp/auth.php +++ b/phpBB/includes/acp/auth.php @@ -113,7 +113,7 @@ class auth_admin extends auth while ($row = $db->sql_fetchrow($result)) { - $forum_ids[] = $row['forum_id']; + $forum_ids[] = (int) $row['forum_id']; } $db->sql_freeresult($result); } diff --git a/phpBB/includes/auth.php b/phpBB/includes/auth.php index 03f2a92ef8..8dd15fea64 100644 --- a/phpBB/includes/auth.php +++ b/phpBB/includes/auth.php @@ -453,10 +453,15 @@ class auth $this->role_cache = array(); while ($row = $db->sql_fetchrow($result)) { - $this->role_cache[$row['role_id']][$row['auth_option_id']] = (bool) $row['auth_setting']; + $this->role_cache[$row['role_id']][$row['auth_option_id']] = (int) $row['auth_setting']; } $db->sql_freeresult($result); + foreach ($this->role_cache as $role_id => $role_options) + { + $this->role_cache[$role_id] = serialize($role_options); + } + $cache->put('_role_cache', $this->role_cache); // Now empty user permissions @@ -747,10 +752,15 @@ class auth while ($row = $db->sql_fetchrow($result)) { - $this->role_cache[$row['role_id']][$row['auth_option_id']] = (bool) $row['auth_setting']; + $this->role_cache[$row['role_id']][$row['auth_option_id']] = (int) $row['auth_setting']; } $db->sql_freeresult($result); + foreach ($this->role_cache as $role_id => $role_options) + { + $this->role_cache[$role_id] = serialize($role_options); + } + $cache->put('_role_cache', $this->role_cache); } @@ -767,7 +777,7 @@ class auth // If a role is assigned, assign all options included within this role. Else, only set this one option. if ($row['auth_role_id']) { - $hold_ary[$row['forum_id']] = (empty($hold_ary[$row['forum_id']])) ? $this->role_cache[$row['auth_role_id']] : $hold_ary[$row['forum_id']] + $this->role_cache[$row['auth_role_id']]; + $hold_ary[$row['forum_id']] = (empty($hold_ary[$row['forum_id']])) ? unserialize($this->role_cache[$row['auth_role_id']]) : $hold_ary[$row['forum_id']] + unserialize($this->role_cache[$row['auth_role_id']]); } else { @@ -792,7 +802,7 @@ class auth } else { - foreach ($this->role_cache[$row['auth_role_id']] as $option_id => $setting) + foreach (unserialize($this->role_cache[$row['auth_role_id']]) as $option_id => $setting) { $this->_set_group_hold_ary($hold_ary[$row['forum_id']], $option_id, $setting); }