#898360 by sun, catch: Cache node types - saves 7% in MySQL in sites using memcache for caching backend

4 files changed, 93 insertions, 47 deletions

diff --git a/modules/node/node.api.php b/modules/node/node.api.php
index 41dcdcae7..6e215e33a 100644
--- a/modules/node/node.api.php
+++ b/modules/node/node.api.php
@@ -143,6 +143,38 @@
  * module's responsibility to provide appropriate realms to limit access to
  * unpublished content.
  *
+ * Node access records are stored in the {node_access} table and define which
+ * grants are required to access a node. There is a special case for the view
+ * operation -- a record with node ID 0 corresponds to a "view all" grant for
+ * the realm and grant ID of that record. If there are no node access modules
+ * enabled, the core node module adds a node ID 0 record for realm 'all'. Node
+ * access modules can also grant "view all" permission on their custom realms;
+ * for example, a module could create a record in {node_access} with:
+ * @code
+ * $record = array(
+ *   'nid' => 0,
+ *   'gid' => 888,
+ *   'realm' => 'example_realm',
+ *   'grant_view' => 1,
+ *   'grant_update' => 0,
+ *   'grant_delete' => 0,
+ * );
+ * drupal_write_record('node_access', $record);
+ * @endcode
+ * And then in its hook_node_grants() implementation, it would need to return:
+ * @code
+ * if ($op == 'view') {
+ *   $grants['example_realm'] = array(888);
+ * }
+ * @endcode
+ * If you decide to do this, be aware that the node_access_rebuild() function
+ * will erase any node ID 0 entry when it is called, so you will need to make
+ * sure to restore your {node_access} record after node_access_rebuild() is
+ * called.
+ *
+ * @see node_access_view_all_nodes()
+ * @see node_access_rebuild()
+ *
  * @param $account
  *   The user object whose grants are requested.
  * @param $op
diff --git a/modules/node/node.module b/modules/node/node.module
index 025be15d3..154119d6a 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -3019,16 +3019,18 @@ function node_access_grants($op, $account = NULL) {
 /**
  * Determines whether the user has a global viewing grant for all nodes.
  *
- * Checks to see whether any module grants 'view' for nid = 0. The node module
- * provides this record if no node access modules are enabled. Other modules
- * can replicate this behavior by providing their own conditional grant for
- * nid = 0. For example, hook_node_grants() can return the following array to
- * give the 'view' privilege to all nodes:
- * @code
- * if ($op == 'view') {
- *   $grants['example_realm'] = array(0);
- * }
- * @endcode
+ * Checks to see whether any module grants global 'view' access to a user
+ * account; global 'view' access is encoded in the {node_access} table as a
+ * grant with nid=0. If no node access modules are enabled, node.module defines
+ * such a global 'view' access grant.
+ *
+ * This function is called when a node listing query is tagged with
+ * 'node_access'; when this function returns TRUE, no node access joins are
+ * added to the query.
+ *
+ * @param $account
+ *   The user object for the user whose access is being checked. If omitted,
+ *   the current user is used.
  *
  * @return
  *   TRUE if 'view' access to all nodes is granted, FALSE otherwise.
@@ -3036,40 +3038,47 @@ function node_access_grants($op, $account = NULL) {
  * @see hook_node_grants()
  * @see _node_query_node_access_alter()
  */
-function node_access_view_all_nodes() {
-  $access = &drupal_static(__FUNCTION__);
+function node_access_view_all_nodes($account = NULL) {
+  global $user;
+  if (!$account) {
+    $account = $user;
+  }
 
-  if (!isset($access)) {
-    // If no modules implement the node access system, access is always TRUE.
-    if (!module_implements('node_grants')) {
-      $access = TRUE;
-    }
-    else {
-      $query = db_select('node_access');
-      $query->addExpression('COUNT(*)');
-      $query
-        ->condition('nid', 0)
-        ->condition('grant_view', 1, '>=');
+  // Statically cache results in an array keyed by $account->uid.
+  $access = &drupal_static(__FUNCTION__);
+  if (isset($access[$account->uid])) {
+    return $access[$account->uid];
+  }
 
-      $grants = db_or();
-      foreach (node_access_grants('view') as $realm => $gids) {
-        foreach ($gids as $gid) {
-          $grants->condition(db_and()
-            ->condition('gid', $gid)
-            ->condition('realm', $realm)
-          );
-        }
-      }
-      if (count($grants) > 0 ) {
-        $query->condition($grants);
-      }
-      $access = $query
-        ->execute()
-        ->fetchField();
+  // If no modules implement the node access system, access is always TRUE.
+  if (!module_implements('node_grants')) {
+    $access[$account->uid] = TRUE;
+  }
+  else {
+    $query = db_select('node_access');
+    $query->addExpression('COUNT(*)');
+    $query
+      ->condition('nid', 0)
+      ->condition('grant_view', 1, '>=');
+
+    $grants = db_or();
+    foreach (node_access_grants('view', $account) as $realm => $gids) {
+      foreach ($gids as $gid) {
+        $grants->condition(db_and()
+          ->condition('gid', $gid)
+          ->condition('realm', $realm)
+        );
       }
+    }
+    if (count($grants) > 0 ) {
+      $query->condition($grants);
+    }
+    $access[$account->uid] = $query
+      ->execute()
+      ->fetchField();
   }
 
-  return $access;
+  return $access[$account->uid];
 }
 
 
@@ -3120,16 +3129,16 @@ function _node_query_node_access_alter($query, $base_table, $type) {
     $op = 'view';
   }
 
-  // If $account can bypass node access, or there are no node access
-  // modules, we don't need to alter the query.
+  // If $account can bypass node access, or there are no node access modules,
+  // or the operation is 'view' and the $acount has a global view grant (i.e.,
+  // a view grant for node ID 0), we don't need to alter the query.
   if (user_access('bypass node access', $account)) {
     return;
   }
   if (!count(module_implements('node_grants'))) {
     return;
   }
-  // If viewing nodes, make sure access rules should be enforced.
-  if ($op == 'view' && node_access_view_all_nodes()) {
+  if ($op == 'view' && node_access_view_all_nodes($account)) {
     return;
   }
 
diff --git a/modules/node/node.test b/modules/node/node.test
index f4d96dcde..df189bc74 100644
--- a/modules/node/node.test
+++ b/modules/node/node.test
@@ -1712,6 +1712,7 @@ class NodeQueryAlter extends DrupalWebTestCase {
     // permission is implemented and granted by the node_access_test module.
     $this->accessUser = $this->drupalCreateUser(array('access content', 'node test view'));
     $this->noAccessUser = $this->drupalCreateUser(array('access content'));
+    $this->noAccessUser2 = $this->drupalCreateUser(array('access content'));
   }
 
   /**
@@ -1840,8 +1841,12 @@ class NodeQueryAlter extends DrupalWebTestCase {
     }
 
     // Have node_test_node_grants return a node_access_all privilege,
-    // to grant the noAccessUser 'view' access.
-    variable_set('node_test_node_access_all', 1);
+    // to grant the noAccessUser 'view' access.  To verify that
+    // node_access_view_all_nodes is properly checking the specified
+    // $account instead of the global $user, we will log in as
+    // noAccessUser2.
+    $this->drupalLogin($this->noAccessUser2);
+    variable_set('node_test_node_access_all_uid', $this->noAccessUser->uid);
     drupal_static_reset('node_access_view_all_nodes');
     try {
       $query = db_select('node', 'mytab')
@@ -1856,7 +1861,7 @@ class NodeQueryAlter extends DrupalWebTestCase {
     catch (Exception $e) {
       $this->fail(t('Altered query is malformed'));
     }
-    variable_del('node_test_node_access_all');
+    variable_del('node_test_node_access_all_uid');
   }
 }
 
diff --git a/modules/node/tests/node_access_test.module b/modules/node/tests/node_access_test.module
index 33f7a01b4..50fb169c6 100644
--- a/modules/node/tests/node_access_test.module
+++ b/modules/node/tests/node_access_test.module
@@ -16,7 +16,7 @@ function node_access_test_node_grants($account, $op) {
   if ($op == 'view' && user_access('node test view', $account)) {
     $grants['node_access_test'] = array(888);
   }
-  if ($op == 'view' && variable_get('node_test_node_access_all', 0)) {
+  if ($op == 'view' && $account->uid == variable_get('node_test_node_access_all_uid', 0)) {
     $grants['node_access_all'] = array(0);
   }
   return $grants;