? user_permissions.module.patch.fail
? user_permissions.patch
? user_permissions_coding_with_flu_sucks.patch
? sites/drupal7.local
Index: modules/system/system.install
===================================================================
RCS file: /cvs/drupal/drupal/modules/system/system.install,v
retrieving revision 1.243
diff -u -r1.243 system.install
--- modules/system/system.install	17 Mar 2008 16:53:58 -0000	1.243
+++ modules/system/system.install	20 Mar 2008 04:22:19 -0000
@@ -366,11 +366,18 @@
   // This sets uid 1 (superuser). We skip uid 2 but that's not a big problem.
   db_query("UPDATE {users} SET uid = 1 WHERE name = '%s'", 'placeholder-for-uid-1');
 
+  // Built-in roles.
   db_query("INSERT INTO {role} (name) VALUES ('%s')", 'anonymous user');
   db_query("INSERT INTO {role} (name) VALUES ('%s')", 'authenticated user');
 
-  db_query("INSERT INTO {permission} (rid, perm, tid) VALUES (%d, '%s', %d)", 1, 'access content', 0);
-  db_query("INSERT INTO {permission} (rid, perm, tid) VALUES (%d, '%s', %d)", 2, 'access comments, access content, post comments, post comments without approval', 0);
+  // Anonymous role permissions.
+  db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 1, 'access content');
+
+  // Authenticated role permissions.
+  db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 2, 'access comments');
+  db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 2, 'access content');
+  db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 2, 'post comments');
+  db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", 2, 'post comments without approval');
 
   db_query("INSERT INTO {variable} (name, value) VALUES ('%s', '%s')", 'theme_default', 's:7:"garland";');
   db_query("UPDATE {system} SET status = %d WHERE type = '%s' AND name = '%s'", 1, 'theme', 'garland');
@@ -2659,8 +2666,38 @@
 }
 
 /**
- * @} End of "defgroup updates-6.x-to-7.x"
- * The next series of updates should start at 8000.
+ * Convert to new method of storing permissions.
  */
+function system_update_7001() {
+  $ret = array();
 
+  db_create_table($ret, 'role_permission', array(
+    'fields' => array(
+      'rid' => array('type' => 'int', 'unsigned' => TRUE, 'not null' => TRUE),
+      // @TODO: Is this the correct max length for a role name?
+      'permission' => array('type' => 'varchar', 'length' => 255, 'not null' => TRUE, 'default' => ''),
+    ),
+    'primary key' => array('rid', 'permission'),
+    'indexes' => array(
+      'permission' => array('permission'),
+    ),
+  ));
 
+  // Copy the permissions from the old {permission} table to the new {role_permission} table.
+  $result = db_query("SELECT rid, perm FROM {permission} ORDER BY rid");
+  while ($role = db_fetch_object($result)) {
+    $permissions = explode(', ', $role->perm);
+    foreach ($permissions as $permission) {
+      db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", $role->rid, $permission);
+    }
+  }
+
+  db_drop_table($ret, 'permission');
+
+  return $ret;
+}
+
+/**
+ * @} End of "defgroup updates-6.x-to-7.x"
+ * The next series of updates should start at 8000.
+ */
Index: modules/user/user.admin.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.admin.inc,v
retrieving revision 1.19
diff -u -r1.19 user.admin.inc
--- modules/user/user.admin.inc	20 Feb 2008 13:46:43 -0000	1.19
+++ modules/user/user.admin.inc	20 Mar 2008 04:22:19 -0000
@@ -494,18 +494,15 @@
  */
 function user_admin_perm($form_state, $rid = NULL) {
   if (is_numeric($rid)) {
-    $result = db_query('SELECT r.rid, p.perm FROM {role} r LEFT JOIN {permission} p ON r.rid = p.rid WHERE r.rid = %d', $rid);
+    $result = db_query('SELECT r.rid, p.permission FROM {role} r LEFT JOIN {role_permission} p ON r.rid = p.rid WHERE r.rid = %d', $rid);
   }
   else {
-    $result = db_query('SELECT r.rid, p.perm FROM {role} r LEFT JOIN {permission} p ON r.rid = p.rid ORDER BY name');
+    $result = db_query('SELECT r.rid, p.permission FROM {role} r LEFT JOIN {role_permission} p ON r.rid = p.rid ORDER BY name');
   }
 
-  // Compile role array:
-  // Add a comma at the end so when searching for a permission, we can
-  // always search for "$perm," to make sure we do not confuse
-  // permissions that are substrings of each other.
-  while ($role = db_fetch_object($result)) {
-    $role_permissions[$role->rid] = $role->perm .',';
+  // Compile role array.
+  while ($perm = db_fetch_object($result)) {
+    $role_permissions[$perm->rid][$perm->permission] = 1;
   }
 
   // Retrieve role names for columns.
@@ -537,7 +534,7 @@
         );
         foreach ($role_names as $rid => $name) {
           // Builds arrays for checked boxes for each role
-          if (strpos($role_permissions[$rid], $perm .',') !== FALSE) {
+          if (isset($role_permissions[$rid][$perm])) {
             $status[$rid][] = $perm;
           }
         }
@@ -545,6 +542,8 @@
     }
   }
 
+  $form['checkboxes']['#tree'] = TRUE;
+
   // Have to build checkboxes here after checkbox arrays are built
   foreach ($role_names as $rid => $name) {
     $form['checkboxes'][$rid] = array('#type' => 'checkboxes', '#options' => $options, '#default_value' => isset($status[$rid]) ? $status[$rid] : array());
@@ -556,22 +555,46 @@
 }
 
 function user_admin_perm_submit($form, &$form_state) {
-  // Save permissions:
-  $result = db_query('SELECT * FROM {role}');
-  while ($role = db_fetch_object($result)) {
-    if (isset($form_state['values'][$role->rid])) {
-      // Delete, so if we clear every checkbox we reset that role;
-      // otherwise permissions are active and denied everywhere.
-      db_query('DELETE FROM {permission} WHERE rid = %d', $role->rid);
-      $form_state['values'][$role->rid] = array_filter($form_state['values'][$role->rid]);
-      if (count($form_state['values'][$role->rid])) {
-        db_query("INSERT INTO {permission} (rid, perm) VALUES (%d, '%s')", $role->rid, implode(', ', array_keys($form_state['values'][$role->rid])));
+  /**
+   * @TODO: This way allows for showing a subset of the available permissions,
+   * and only affecting those with changes. This may or may not actually be desired.
+   * If it is decided that this isn't needed behavior, we can get by with just
+   * wiping everything by rid.
+   */
+
+  foreach ($form_state['values']['checkboxes'] as $rid => $value) {
+    $checked = array_filter($value);
+    $permissions = array_keys($form['checkboxes'][$rid]['#options']);
+    foreach ($permissions as $permission) {
+      // Delete first, so we can easily "unset" permissions when unchecking.
+      db_query("DELETE FROM {role_permission} WHERE rid = %d AND permission = '%s'", $rid, $permission);
+      if (isset($checked[$permission])) {
+        db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", $rid, $permission);
       }
     }
   }
 
+  /**
+   * @TODO: And here's the other way.
+   * This assumes that all checkboxes are being shown to the admin.
+   * (The old way had this assumption.)
+   */
+/*
+  foreach ($form_state['values']['checkboxes'] as $rid => $value) {
+    $checked = array_keys(array_filter($value));
+    // Delete existing permissions for the role. This handles "unchecking" checkboxes.
+    db_query('DELETE FROM {role_permission} WHERE rid = %d', $rid);
+    foreach ($checked as $permission) {
+      db_query("INSERT INTO {role_permission} (rid, permission) VALUES (%d, '%s')", $rid, $permission);
+    }
+  }
+*/
+
   drupal_set_message(t('The changes have been saved.'));
 
+  // Clear cached permissions.
+  cache_clear_all('role_perm:', 'cache', TRUE);
+
   // Clear the cached pages
   cache_clear_all();
 }
@@ -697,7 +720,7 @@
   }
   else if ($form_state['values']['op'] == t('Delete role')) {
     db_query('DELETE FROM {role} WHERE rid = %d', $form_state['values']['rid']);
-    db_query('DELETE FROM {permission} WHERE rid = %d', $form_state['values']['rid']);
+    db_query('DELETE FROM {role_permission} WHERE rid = %d', $form_state['values']['rid']);
     // Update the users who have this role set:
     db_query('DELETE FROM {users_roles} WHERE rid = %d', $form_state['values']['rid']);
 
Index: modules/user/user.install
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.install,v
retrieving revision 1.7
diff -u -r1.7 user.install
--- modules/user/user.install	15 Mar 2008 12:31:29 -0000	1.7
+++ modules/user/user.install	20 Mar 2008 04:22:20 -0000
@@ -74,38 +74,26 @@
     'primary key' => array('aid'),
   );
 
-  $schema['permission'] = array(
-    'description' => t('Stores permissions for users.'),
+  $schema['role_permission'] = array(
+    'description' => t('Stores the permissions assigned to user roles.'),
     'fields' => array(
-      'pid' => array(
-        'type' => 'serial',
-        'not null' => TRUE,
-        'description' => t('Primary Key: Unique permission ID.'),
-      ),
       'rid' => array(
         'type' => 'int',
         'unsigned' => TRUE,
         'not null' => TRUE,
-        'default' => 0,
-        'description' => t('The {role}.rid to which the permissions are assigned.'),
-      ),
-      'perm' => array(
-        'type' => 'text',
-        'not null' => FALSE,
-        'size' => 'big',
-        'description' => t('List of permissions being assigned.'),
+        'description' => t('Foreign Key: Role id.'),
       ),
-      'tid' => array(
-        'type' => 'int',
-        'unsigned' => TRUE,
+      'permission' => array(
+        'type' => 'varchar',
+        'length' => 255, /**@ @todo: Is this the correct max length for a role name? */
         'not null' => TRUE,
-        'default' => 0,
-        'description' => t('Originally intended for taxonomy-based permissions, but never used.'),
+        'default' => '',
+        'description' => t('Permission name.'),
       ),
     ),
-    'primary key' => array('pid'),
+    'primary key' => array('rid', 'permission'),
     'indexes' => array(
-      'rid' => array('rid'),
+      'permission' => array('permission'),
     ),
   );
 
Index: modules/user/user.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.module,v
retrieving revision 1.897
diff -u -r1.897 user.module
--- modules/user/user.module	19 Mar 2008 07:35:15 -0000	1.897
+++ modules/user/user.module	20 Mar 2008 04:22:20 -0000
@@ -464,13 +464,27 @@
   // To reduce the number of SQL queries, we cache the user's permissions
   // in a static variable.
   if (!isset($perm[$account->uid])) {
-    $result = db_query("SELECT p.perm FROM {role} r INNER JOIN {permission} p ON p.rid = r.rid WHERE r.rid IN (". db_placeholders($account->roles) .")", array_keys($account->roles));
+    $roles = array_keys($account->roles);
+    sort($roles);
+    $cache = cache_get('role_perm:'. implode(',', $roles));
+    if (!$cache) {
+      // If we don't have a cached copy of the permissions array for this
+      // combination of roles, generate it.
+      // @todo: Is it actually worth putting DISTINCT here? Multiple roles granting a permission
+      // is perfectly OK code-wise, it's just a balance of restultset size vs the amount of work the db has to do
+      // vs the amount of time we spend setting a variable to true more than once...
+      $result = db_query('SELECT DISTINCT permission from {role_permission} WHERE rid IN ('. db_placeholders($roles) .')', $roles);
+      $cache = array();
+      while ($row = db_fetch_object($result)) {
+        $cache[$row->permission] = 1;
+      }
 
-    $perms = array();
-    while ($row = db_fetch_object($result)) {
-      $perms += array_flip(explode(', ', $row->perm));
+      cache_set('role_perm:'. implode(',', $roles), $cache);
+    }
+    else {
+      $cache = $cache->data;
     }
-    $perm[$account->uid] = $perms;
+    $perm[$account->uid] = $cache;
   }
 
   return isset($perm[$account->uid][$string]);
@@ -1634,7 +1648,7 @@
   );
 
   if (!empty($permission)) {
-    $result = db_query("SELECT r.* FROM {role} r INNER JOIN {permission} p ON r.rid = p.rid WHERE p.perm LIKE '%%%s%%' ORDER BY r.name", $permission);
+    $result = db_query("SELECT r.* FROM {role} r INNER JOIN {role_permission} p ON r.rid = p.rid WHERE p.permission = '%s' ORDER BY r.name", $permission);
   }
   else {
     $result = db_query('SELECT * FROM {role} ORDER BY name');
@@ -1900,8 +1914,8 @@
   ksort($options);
   $filters['permission'] = array(
     'title' => t('permission'),
-    'join' => 'LEFT JOIN {permission} p ON ur.rid = p.rid',
-    'where' => " ((p.perm IS NOT NULL AND p.perm LIKE '%%%s%%') OR u.uid = 1) ",
+    'join' => 'LEFT JOIN {role_permission} p ON ur.rid = p.rid',
+    'where' => " ((p.permission IS NOT NULL AND p.permission = '%s') OR u.uid = 1) ",
     'options' => $options,
   );