#560746 follow-up by chx: Split hook_block_info_alter() into list_ and info_alter() and clean up block_rehash().

5 files changed, 114 insertions, 75 deletions

diff --git a/modules/block/block.api.php b/modules/block/block.api.php
index 7a31a1b45..96c92668e 100644
--- a/modules/block/block.api.php
+++ b/modules/block/block.api.php
@@ -70,6 +70,25 @@ function hook_block_info() {
 }
 
 /**
+ * Change block definition before saving to the database.
+ *
+ * @param $blocks
+ *   A multidimensional array of blocks keyed by the defining module and delta
+ *   the value is a block as seen in hook_block_info(). This hook is fired
+ *   after the blocks are collected from hook_block_info() and the database,
+ *   right before saving back to the database.
+ * @param $theme
+ *   The theme these blocks belong to.
+ * @param $code_blocks
+ *   The blocks as defined in hook_block_info before overwritten by the
+ *   database data.
+ */
+function hook_block_info_alter(&$blocks, $theme, $code_blocks) {
+  // Disable the login block.
+  $blocks['user']['login']['status'] = 0;
+}
+
+/**
  * Configuration form for the block.
  *
  * @param $delta
@@ -240,7 +259,7 @@ function hook_block_view_MODULE_DELTA_alter(&$data, $block) {
  * This example shows how to achieve language specific visibility setting for
  * blocks.
  */
-function hook_block_info_alter(&$blocks) {
+function hook_block_list_alter(&$blocks) {
   global $language, $theme_key;
 
   $result = db_query('SELECT module, delta, language FROM {my_table}');
diff --git a/modules/block/block.module b/modules/block/block.module
index 83ed3b922..13140de00 100644
--- a/modules/block/block.module
+++ b/modules/block/block.module
@@ -317,80 +317,100 @@ function _block_rehash($theme = NULL) {
   global $theme_key;
 
   drupal_theme_initialize();
-
   if (!isset($theme)) {
     // If theme is not specifically set, rehash for the current theme.
     $theme = $theme_key;
   }
-
-  $old_blocks = array();
-  $result = db_query("SELECT * FROM {block} WHERE theme = :theme", array(':theme' => $theme));
-  foreach ($result as $old_block) {
-    $old_block = is_object($old_block) ? get_object_vars($old_block) : $old_block;
-    $old_blocks[$old_block['module']][$old_block['delta']] = $old_block;
-  }
-
-  $blocks = array();
-  // Valid region names for the theme.
   $regions = system_region_list($theme);
 
+  // These are the blocks the function will return.
+  $blocks = array();
+  // These are the blocks defined by code and modified by the database.
+  $current_blocks = array();
+  // These are {block}.bid values to be kept.
+  $bids = array();
+  $or = db_or();
+  // Gather the blocks defined by modules.
   foreach (module_implements('block_info') as $module) {
     $module_blocks = module_invoke($module, 'block_info');
-    if ($module_blocks) {
-      foreach ($module_blocks as $delta => $block) {
-        if (empty($old_blocks[$module][$delta])) {
-          // If it's a new block, add identifiers.
-          $block['module'] = $module;
-          $block['delta']  = $delta;
-          $block['theme']  = $theme;
-          if (!isset($block['pages'])) {
-            // {block}.pages is type 'text', so it cannot have a
-            // default value, and not null, so we need to provide
-            // value if the module did not.
-            $block['pages']  = '';
-          }
-          // Add defaults and save it into the database.
-          drupal_write_record('block', $block);
-          // Set region to none if not enabled.
-          $block['region'] = $block['status'] ? $block['region'] : BLOCK_REGION_NONE;
-          // Add to the list of blocks we return.
-          $blocks[] = $block;
-        }
-        else {
-          // If it's an existing block, database settings should overwrite
-          // the code. But aside from 'info' everything that's definable in
-          // code is stored in the database and we do not store 'info', so we
-          // do not need to update the database here.
-          // Add 'info' to this block.
-          $old_blocks[$module][$delta]['info'] = $block['info'];
-          // If the region name does not exist, disable the block and assign it to none.
-          if (!empty($old_blocks[$module][$delta]['region']) && !isset($regions[$old_blocks[$module][$delta]['region']])) {
-            drupal_set_message(t('The block %info was assigned to the invalid region %region and has been disabled.', array('%info' => $old_blocks[$module][$delta]['info'], '%region' => $old_blocks[$module][$delta]['region'])), 'warning');
-            $old_blocks[$module][$delta]['status'] = 0;
-            $old_blocks[$module][$delta]['region'] = BLOCK_REGION_NONE;
-          }
-          else {
-            $old_blocks[$module][$delta]['region'] = $old_blocks[$module][$delta]['status'] ? $old_blocks[$module][$delta]['region'] : BLOCK_REGION_NONE;
-          }
-          // Add this block to the list of blocks we return.
-          $blocks[] = $old_blocks[$module][$delta];
-          // Remove this block from the list of blocks to be deleted.
-          unset($old_blocks[$module][$delta]);
-        }
-      }
+    foreach ($module_blocks as $delta => $block) {
+      // Compile a condition to retrieve this block from the database.
+      $condition = db_and()
+        ->condition('module', $module)
+        ->condition('delta', $delta);
+      $or->condition($condition);
+      // Add identifiers.
+      $block['module'] = $module;
+      $block['delta']  = $delta;
+      $block['theme']  = $theme;
+      $current_blocks[$module][$delta] = $block;
     }
   }
-
-  // Remove blocks that are no longer defined by the code from the database.
-  foreach ($old_blocks as $module => $old_module_blocks) {
-    foreach ($old_module_blocks as $delta => $block) {
-      db_delete('block')
-        ->condition('module', $module)
-        ->condition('delta', $delta)
-        ->condition('theme', $theme)
-        ->execute();
+  // Save the blocks defined in code for alter context.
+  $code_blocks = $current_blocks;
+  $database_blocks = db_select('block', 'b')
+    ->fields('b')
+    ->condition($or)
+    ->condition('theme', $theme)
+    ->execute();
+  foreach ($database_blocks as $block) {
+    // Preserve info which is not in the database.
+    $block->info = $current_blocks[$block->module][$block->delta]['info'];
+    // Blocks stored in the database override the blocks defined in code.
+    $current_blocks[$block->module][$block->delta] = get_object_vars($block);
+    // Preserve this block.
+    $bids[$block->bid] = $block->bid;
+  }
+  drupal_alter('block_info', $current_blocks, $theme, $code_blocks);
+  foreach ($current_blocks as $module => $module_blocks) {
+    foreach ($module_blocks as $delta => $block) {
+      if (!isset($block['pages'])) {
+        // {block}.pages is type 'text', so it cannot have a
+        // default value, and not null, so we need to provide
+        // value if the module did not.
+        $block['pages']  = '';
+      }
+      // Make sure weight is set.
+      if (!isset($block['weight'])) {
+        $block['weight'] = 0;
+      }
+      if (!empty($block['region']) && $block['region'] != BLOCK_REGION_NONE && !isset($regions[$block['region']])) {
+        drupal_set_message(t('The block %info was assigned to the invalid region %region and has been disabled.', array('%info' => $block['info'], '%region' => $block['region'])), 'warning');
+        // Disabled modules are moved into the BLOCK_REGION_NONE later so no
+        // need to move the bock to another region.
+        $block['status'] = 0;
+      }
+      // Set region to none if not enabled and make sure status is set.
+      if (empty($block['status'])) {
+        $block['status'] = 0;
+        $block['region'] = BLOCK_REGION_NONE;
+      }
+      // There is no point saving disabled blocks. Still, we need to save them
+      // beecause the 'title' attribute is saved to the {blocks} table.
+      if (isset($block['bid'])) {
+        // If the block has a bid property, it comes from the database and
+        // the record needs to be updated, so set the primary key to 'bid'
+        // before passing to drupal_write_record().
+        $primary_keys = array('bid');
+        // Remove a block from the list of blocks to keep if it became disabled.
+        unset($bids[$block['bid']]);
+      }
+      else {
+        $primary_keys = array();
+      }
+      drupal_write_record('block', $block, $primary_keys);
+      // Add to the list of blocks we return.
+      $blocks[] = $block;
     }
   }
+  if ($bids) {
+    // Remove disabled that are no longer defined by the code from the
+    // database.
+    db_delete('block')
+      ->condition('bid', $bids, 'NOT IN')
+      ->condition('theme', $theme)
+      ->execute();
+  }
   return $blocks;
 }
 
@@ -624,7 +644,7 @@ function _block_load_blocks() {
 
   $block_info = $result->fetchAllAssoc('bid');
   // Allow modules to modify the block list.
-  drupal_alter('block_info', $block_info);
+  drupal_alter('block_list', $block_info);
 
   $blocks = array();
   foreach ($block_info as $block) {
@@ -634,12 +654,12 @@ function _block_load_blocks() {
 }
 
 /**
- * Implements hook_block_info_alter().
+ * Implements hook_block_list_alter().
  *
  * Check the page, user role and user specific visibilty settings.
  * Remove the block if the visibility conditions are not met.
  */
-function block_block_info_alter(&$blocks) {
+function block_block_list_alter(&$blocks) {
   global $user, $theme_key;
 
   // Build an array of roles for each block.
diff --git a/modules/dashboard/dashboard.module b/modules/dashboard/dashboard.module
index 1ef9ef195..75d434cbe 100644
--- a/modules/dashboard/dashboard.module
+++ b/modules/dashboard/dashboard.module
@@ -73,13 +73,13 @@ function dashboard_menu_alter(&$items) {
 }
 
 /**
- * Implements hook_block_info_alter().
+ * Implements hook_block_list_alter().
  *
  * Skip rendering dashboard blocks when not on the dashboard page itself. This
  * prevents expensive dashboard blocks from causing performance issues on pages
  * where they will never be displayed.
  */
-function dashboard_block_info_alter(&$blocks) {
+function dashboard_block_list_alter(&$blocks) {
   if (!dashboard_is_visible()) {
     foreach ($blocks as $key => $block) {
       if (in_array($block->region, dashboard_regions())) {
diff --git a/modules/node/node.module b/modules/node/node.module
index ee418bda8..f2e566ddd 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -2293,12 +2293,12 @@ function node_modules_uninstalled($modules) {
 }
 
 /**
- * Implements hook_block_info_alter().
+ * Implements hook_block_list_alter().
  *
  * Check the content type specific visibilty settings.
  * Remove the block if the visibility conditions are not met.
  */
-function node_block_info_alter(&$blocks) {
+function node_block_list_alter(&$blocks) {
   global $theme_key;
 
   // Build an array of node types for each block.
diff --git a/modules/overlay/overlay.module b/modules/overlay/overlay.module
index 1959be5e2..8215c5ad6 100644
--- a/modules/overlay/overlay.module
+++ b/modules/overlay/overlay.module
@@ -223,9 +223,9 @@ function overlay_page_alter(&$page) {
 }
 
 /**
- * Implements hook_block_info_alter().
+ * Implements hook_block_list_alter().
  */
-function overlay_block_info_alter(&$blocks) {
+function overlay_block_list_alter(&$blocks) {
   // If we are limiting rendering to a subset of page regions, hide all blocks
   // which appear in regions not on that list. Note that overlay_page_alter()
   // does a more comprehensive job of preventing unwanted regions from being
@@ -666,7 +666,7 @@ function _overlay_region_list($type) {
  *   and all regions of the page will be rendered.
  *
  * @see overlay_page_alter()
- * @see overlay_block_info_alter()
+ * @see overlay_block_list_alter()
  * @see overlay_set_regions_to_render()
  */
 function overlay_get_regions_to_render() {
@@ -688,7 +688,7 @@ function overlay_get_regions_to_render() {
  *   are not being limited.
  *
  * @see overlay_page_alter()
- * @see overlay_block_info_alter()
+ * @see overlay_block_list_alter()
  * @see overlay_get_regions_to_render()
  */
 function overlay_set_regions_to_render($regions = NULL) {
@@ -716,7 +716,7 @@ function overlay_set_regions_to_render($regions = NULL) {
  */
 function overlay_render_region($region) {
   // Indicate the region that we will be rendering, so that other regions will
-  // be hidden by overlay_page_alter() and overlay_block_info_alter().
+  // be hidden by overlay_page_alter() and overlay_block_list_alter().
   overlay_set_regions_to_render(array($region));
   // Do what is necessary to force drupal_render_page() to only display HTML
   // from the requested region. Specifically, declare that the main page