Issue #918808 by moshe weitzman, Damien Tournoud, dww: Standardize block cache...

Issue #918808 by moshe weitzman, Damien Tournoud, dww: Standardize block cache as a drupal_render() #cache.

modules/block/block.install

@@ -185,3 +185,21 @@ function block_install() {
     ->condition('name', 'block')
     ->execute();
 }
+
+/**
+ * @defgroup updates-7.x-to-8.x Updates from 7.x to 8.x
+ * @{
+ * Update functions from 7.x to 8.x.
+ */
+
+/**
+ * Block cache is always enabled in 8.x.
+ */
+function block_update_8000() {
+  variable_del('block_cache');
+}
+
+/**
+ * @} End of "defgroup updates-7.x-to-8.x"
+ * The next series of updates should start at 9000.
+ */

modules/block/block.module

@@ -313,7 +313,7 @@ function block_page_build(&$page) {
 function block_get_blocks_by_region($region) {
   $build = array();
   if ($list = block_list($region)) {
-    $build = _block_get_renderable_array($list);
+    $build = _block_get_renderable_region($list);
   }
   return $build;
 }
@@ -326,12 +326,43 @@ function block_get_blocks_by_region($region) {
  * @return
  *   A renderable array.
  */
-function _block_get_renderable_array($list = array()) {
+function _block_get_renderable_region($list = array()) {
   $weight = 0;
   $build = array();
   foreach ($list as $key => $block) {
-    $build[$key] = $block->content;
-    unset($block->content);
+    $build[$key] = array(
+      '#block' => $block,
+      '#weight' => ++$weight,
+      '#theme_wrappers' => array('block'),
+    );
+
+    // Block caching is not compatible with node_access modules. We also
+    // preserve the submission of forms in blocks, by fetching from cache
+    // only if the request method is 'GET' (or 'HEAD'). User 1 being out of
+    // the regular 'roles define permissions' schema, it brings too many
+    // chances of having unwanted output get in the cache and later be served
+    // to other users. We therefore exclude user 1 from block caching.
+    if (
+      $GLOBALS['user']->uid == 1 ||
+      count(module_implements('node_grants')) ||
+      !in_array($_SERVER['REQUEST_METHOD'], array('GET', 'HEAD')) ||
+      in_array($block->cache, array(DRUPAL_NO_CACHE, DRUPAL_CACHE_CUSTOM))
+    ) {
+      // Non-cached blocks get built immediately. Provides more content
+      // that can be easily manipulated during hook_page_alter().
+      $build[$key] = _block_get_renderable_block($build[$key]);
+    }
+    else {
+      $build[$key] += array(
+        '#pre_render' => array('_block_get_renderable_block'),
+        '#cache' => array(
+          'keys' => array($block->module, $block->delta),
+          'granularity' => $block->cache,
+          'bin' => 'block',
+          'expire' => CACHE_TEMPORARY,
+        ),
+      );
+    }
 
     // Add contextual links for this block; skip the main content block, since
     // contextual links are basically output as tabs/local tasks already. Also
@@ -341,12 +372,6 @@ function _block_get_renderable_array($list = array()) {
     if ($key != 'system_main' && $key != 'system_help') {
       $build[$key]['#contextual_links']['block'] = array('admin/structure/block/manage', array($block->module, $block->delta));
     }
-
-    $build[$key] += array(
-      '#block' => $block,
-      '#weight' => ++$weight,
-    );
-    $build[$key]['#theme_wrappers'][] ='block';
   }
   $build['#sorted'] = TRUE;
   return $build;
@@ -654,9 +679,6 @@ function block_list($region) {
   if (!isset($blocks[$region])) {
     $blocks[$region] = array();
   }
-  else {
-    $blocks[$region] = _block_render_blocks($blocks[$region]);
-  }
 
   return $blocks[$region];
 }
@@ -696,6 +718,7 @@ function _block_load_blocks() {
   global $theme_key;
 
   $query = db_select('block', 'b');
+  $query->addField('b', 'title', 'subject');
   $result = $query
     ->fields('b')
     ->condition('b.theme', $theme_key)
@@ -808,45 +831,39 @@ function block_block_list_alter(&$blocks) {
 }
 
 /**
- * Render the content and subject for a set of blocks.
- *
- * @param $region_blocks
- *   An array of block objects such as returned for one region by _block_load_blocks().
+ * Build the content and subject for a block. For cacheable blocks, this is
+ * called during #pre_render.
  *
+ * @param $element
+ *   A renderable array.
  * @return
- *   An array of visible blocks as expected by drupal_render().
+ *   A renderable array.
  */
-function _block_render_blocks($region_blocks) {
-  foreach ($region_blocks as $key => $block) {
+function _block_get_renderable_block($element) {
+  $block = $element['#block'];
+
   // Render the block content if it has not been created already.
   if (!isset($block->content)) {
-      // Erase the block from the static array - we'll put it back if it has
-      // content.
-      unset($region_blocks[$key]);
-      // Try fetching the block from cache. Block caching is not compatible
-      // with node_access modules. We also preserve the submission of forms in
-      // blocks, by fetching from cache only if the request method is 'GET'
-      // (or 'HEAD').
-      if (!count(module_implements('node_grants')) && ($_SERVER['REQUEST_METHOD'] == 'GET' || $_SERVER['REQUEST_METHOD'] == 'HEAD') && ($cid = _block_get_cache_id($block)) && ($cache = cache('block')->get($cid))) {
-        $array = $cache->data;
-      }
-      else {
     $array = module_invoke($block->module, 'block_view', $block->delta);
 
     // Allow modules to modify the block before it is viewed, via either
     // hook_block_view_alter() or hook_block_view_MODULE_DELTA_alter().
     drupal_alter(array('block_view', "block_view_{$block->module}_{$block->delta}"), $array, $block);
 
-        if (isset($cid)) {
-          cache('block')->set($cid, $array, CACHE_TEMPORARY);
-        }
+    if (empty($array)) {
+      // Blocks without content should emit no markup at all.
+      $element += array(
+        '#access' => FALSE,
+        '#printed' => TRUE,
+      );
     }
-
-      if (isset($array) && is_array($array)) {
+    elseif (isset($array) && is_array($array)) {
       foreach ($array as $k => $v) {
         $block->$k = $v;
       }
     }
+  }
+
   if (isset($block->content) && $block->content) {
     // Normalize to the drupal_render() structure.
     if (is_string($block->content)) {
@@ -858,46 +875,13 @@ function _block_render_blocks($region_blocks) {
       // markup.
       $block->subject = $block->title == '<none>' ? '' : check_plain($block->title);
     }
-        if (!isset($block->subject)) {
-          $block->subject = '';
-        }
-        $region_blocks["{$block->module}_{$block->delta}"] = $block;
-      }
-    }
-  }
-  return $region_blocks;
-}
 
-/**
- * Assemble the cache_id to use for a given block.
- *
- * The cache_id string reflects the viewing context for the current block
- * instance, obtained by concatenating the relevant context information
- * (user, page, ...) according to the block's cache settings (BLOCK_CACHE_*
- * constants). Two block instances can use the same cached content when
- * they share the same cache_id.
- *
- * Theme and language contexts are automatically differentiated.
- *
- * @param $block
- * @return
- *   The string used as cache_id for the block.
- */
-function _block_get_cache_id($block) {
-  global $user;
-
-  // User 1 being out of the regular 'roles define permissions' schema,
-  // it brings too many chances of having unwanted output get in the cache
-  // and later be served to other users. We therefore exclude user 1 from
-  // block caching.
-  if (variable_get('block_cache', FALSE) && !in_array($block->cache, array(DRUPAL_NO_CACHE, DRUPAL_CACHE_CUSTOM)) && $user->uid != 1) {
-    // Start with common sub-patterns: block identification, theme, language.
-    $cid_parts[] = $block->module;
-    $cid_parts[] = $block->delta;
-    $cid_parts = array_merge($cid_parts, drupal_render_cid_parts($block->cache));
-
-    return implode(':', $cid_parts);
+    // Add the content renderable array to the main element.
+    $element['content'] = $block->content;
+    unset($block->content);
+    $element['#block'] = $block;
   }
+  return $element;
 }
 
 /**
@@ -992,21 +976,6 @@ function block_menu_delete($menu) {
     ->execute();
 }
 
-/**
- * Implements hook_form_FORM_ID_alter().
- */
-function block_form_system_performance_settings_alter(&$form, &$form_state) {
-  $disabled = count(module_implements('node_grants'));
-  $form['caching']['block_cache'] = array(
-    '#type' => 'checkbox',
-    '#title' => t('Cache blocks'),
-    '#default_value' => variable_get('block_cache', FALSE),
-    '#disabled' => $disabled,
-    '#description' => $disabled ? t('Block caching is inactive because you have enabled modules defining content access restrictions.') : NULL,
-    '#weight' => -1,
-  );
-}
-
 /**
  * Implements hook_admin_paths().
  */

modules/block/block.test

@@ -513,9 +513,6 @@ class BlockCacheTestCase extends DrupalWebTestCase {
     user_save($this->normal_user_alt, array('roles' => $this->normal_user->roles));
     $this->normal_user_alt->roles = $this->normal_user->roles;
 
-    // Enable block caching.
-    variable_set('block_cache', TRUE);
-
     // Enable our test block.
     $edit['blocks[block_test_test_cache][region]'] = 'sidebar_first';
     $this->drupalPost('admin/structure/block', $edit, t('Save blocks'));

modules/comment/comment.test

@@ -1589,6 +1589,9 @@ class CommentBlockFunctionalTest extends CommentHelperCase {
     // block.
     $this->drupalLogout();
     user_role_revoke_permissions(DRUPAL_ANONYMOUS_RID, array('access comments'));
+    // drupalCreateNode() does not automatically flush content caches unlike
+    // posting a node from a node form.
+    cache_clear_all();
     $this->drupalGet('');
     $this->assertNoText($block['title'], t('Block was not found.'));
     user_role_grant_permissions(DRUPAL_ANONYMOUS_RID, array('access comments'));

modules/dashboard/dashboard.module

@@ -187,7 +187,7 @@ function dashboard_page_build(&$page) {
           }
           $block->subject = t('@title', array('@title' => $block_info[$block->module][$block->delta]['info']));
           $block_render = array($block->module . '_' . $block->delta => $block);
-          $build = _block_get_renderable_array($block_render);
+          $build = _block_get_renderable_region($block_render);
           $page['content']['dashboard'][$block->region][] = $build;
         }
       }
@@ -526,8 +526,7 @@ function dashboard_show_block_content($module, $delta) {
     ->fetchObject();
   $block_object->enabled = $block_object->page_match = TRUE;
   $blocks[$module . "_" . $delta] = $block_object;
-  $block_content = _block_render_blocks($blocks);
-  $build = _block_get_renderable_array($block_content);
+  $build = _block_get_renderable_region($blocks);
   $rendered_block = drupal_render($build);
   print $rendered_block;
   drupal_exit();
@@ -674,4 +673,3 @@ function theme_dashboard_disabled_block($variables) {
   }
   return $output;
 }
-

modules/node/node.test

@@ -1855,6 +1855,9 @@ class NodeBlockFunctionalTest extends DrupalWebTestCase {
 
     // Post an additional node.
     $node4 = $this->drupalCreateNode($default_settings);
+    // drupalCreateNode() does not automatically flush content caches unlike
+    // posting a node from a node form.
+    cache_clear_all();
 
     // Test that all four nodes are shown.
     $this->drupalGet('');