From 08b9bd41723a0a23687c59033c15d66c1840b4f3 Mon Sep 17 00:00:00 2001 From: Nathaniel Date: Thu, 29 Sep 2011 12:29:59 +0900 Subject: [PATCH] Issue #918808 by moshe weitzman, Damien Tournoud, dww: Standardize block cache as a drupal_render() #cache. --- modules/block/block.install | 18 +++ modules/block/block.module | 183 ++++++++++++----------------- modules/block/block.test | 3 - modules/comment/comment.test | 5 +- modules/dashboard/dashboard.module | 6 +- modules/node/node.test | 3 + 6 files changed, 103 insertions(+), 115 deletions(-) diff --git a/modules/block/block.install b/modules/block/block.install index deed13af3d..dea128d5e1 100644 --- a/modules/block/block.install +++ b/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. + */ diff --git a/modules/block/block.module b/modules/block/block.module index 8f882f7a69..d0f07e447b 100644 --- a/modules/block/block.module +++ b/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,96 +831,57 @@ 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) { - // 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); +function _block_get_renderable_block($element) { + $block = $element['#block']; - // 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); + // Render the block content if it has not been created already. + if (!isset($block->content)) { + $array = module_invoke($block->module, 'block_view', $block->delta); - if (isset($cid)) { - cache('block')->set($cid, $array, CACHE_TEMPORARY); - } - } + // 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($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)) { - $block->content = array('#markup' => $block->content); - } - // Override default block title if a custom display title is present. - if ($block->title) { - // Check plain here to allow module generated titles to keep any - // markup. - $block->subject = $block->title == '' ? '' : check_plain($block->title); - } - if (!isset($block->subject)) { - $block->subject = ''; - } - $region_blocks["{$block->module}_{$block->delta}"] = $block; + if (empty($array)) { + // Blocks without content should emit no markup at all. + $element += array( + '#access' => FALSE, + '#printed' => TRUE, + ); + } + elseif (isset($array) && is_array($array)) { + foreach ($array as $k => $v) { + $block->$k = $v; } } } - 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); + if (isset($block->content) && $block->content) { + // Normalize to the drupal_render() structure. + if (is_string($block->content)) { + $block->content = array('#markup' => $block->content); + } + // Override default block title if a custom display title is present. + if ($block->title) { + // Check plain here to allow module generated titles to keep any + // markup. + $block->subject = $block->title == '' ? '' : check_plain($block->title); + } + + // 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(). */ diff --git a/modules/block/block.test b/modules/block/block.test index ed68428c89..a6ab2bf3fe 100644 --- a/modules/block/block.test +++ b/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')); diff --git a/modules/comment/comment.test b/modules/comment/comment.test index 3b83342f41..a825eb563f 100644 --- a/modules/comment/comment.test +++ b/modules/comment/comment.test @@ -1,7 +1,7 @@ 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')); diff --git a/modules/dashboard/dashboard.module b/modules/dashboard/dashboard.module index 7d5d7497b1..1216cc00c1 100644 --- a/modules/dashboard/dashboard.module +++ b/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; } - diff --git a/modules/node/node.test b/modules/node/node.test index a33a353cf6..817f3908e0 100644 --- a/modules/node/node.test +++ b/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(''); -- GitLab