#333658 by sun: Code clean-up for batch.inc.

2 files changed, 138 insertions, 93 deletions

diff --git a/includes/batch.inc b/includes/batch.inc
index b65a80452..de47da277 100644
--- a/includes/batch.inc
+++ b/includes/batch.inc
@@ -1,15 +1,28 @@
 <?php
 // $Id$
 
+
 /**
- * @file Batch processing API for processes to run in multiple HTTP requests.
+ * @file
+ * Batch processing API for processes to run in multiple HTTP requests.
+ *
+ * Please note that batches are usually invoked by form submissions, which is
+ * why the core interaction functions of the batch processing API live in
+ * form.inc.
+ *
+ * @see form.inc
+ * @see batch_set()
+ * @see batch_process()
+ * @see batch_get()
  */
 
 /**
  * State-based dispatcher for the batch processing page.
+ *
+ * @see _batch_shutdown()
  */
 function _batch_page() {
-  $batch =& batch_get();
+  $batch = &batch_get();
 
   if (!isset($_REQUEST['id'])) {
     return FALSE;
@@ -23,10 +36,10 @@ function _batch_page() {
 
   $batch = unserialize($batch);
 
-  // Register database update for end of processing.
+  // Register database update for the end of processing.
   register_shutdown_function('_batch_shutdown');
 
-  // Add batch-specific css.
+  // Add batch-specific CSS.
   foreach ($batch['sets'] as $batch_set) {
     foreach ($batch_set['css'] as $css) {
       drupal_add_css($css);
@@ -41,12 +54,12 @@ function _batch_page() {
       break;
 
     case 'do':
-      // JS-version AJAX callback.
+      // JavaScript-based progress page callback.
       _batch_do();
       break;
 
     case 'do_nojs':
-      // Non-JS progress page.
+      // Non-JavaScript-based progress page.
       $output = _batch_progress_page_nojs();
       break;
 
@@ -59,13 +72,13 @@ function _batch_page() {
 }
 
 /**
- * Initiate the batch processing
+ * Initialize the batch processing.
+ *
+ * JavaScript-enabled clients are identified by the 'has_js' cookie set in
+ * drupal.js. If no JavaScript-enabled page has been visited during the current
+ * user's browser session, the non-JavaScript version is returned.
  */
 function _batch_start() {
-  // Choose between the JS and non-JS version.
-  // JS-enabled users are identified through the 'has_js' cookie set in drupal.js.
-  // If the user did not visit any JS enabled page during his browser session,
-  // he gets the non-JS version...
   if (isset($_COOKIE['has_js']) && $_COOKIE['has_js']) {
     return _batch_progress_page_js();
   }
@@ -75,38 +88,44 @@ function _batch_start() {
 }
 
 /**
- * Batch processing page with JavaScript support.
+ * Output a batch processing page with JavaScript support.
+ *
+ * This initializes the batch and error messages. Note that in JavaScript-based
+ * processing, the batch processing page is displayed only once and updated via
+ * AHAH requests, so only the first batch set gets to define the page title.
+ * Titles specified by subsequent batch sets are not displayed.
+ *
+ * @see batch_set()
+ * @see _batch_do()
  */
 function _batch_progress_page_js() {
   $batch = batch_get();
 
-  // The first batch set gets to set the page title and the initialization and
-  // error messages. Only safe strings should be passed in to batch_set().
   $current_set = _batch_current_set();
   drupal_set_title($current_set['title'], PASS_THROUGH);
-  drupal_add_js('misc/progress.js', array('cache' => FALSE));
 
-  $url = url($batch['url'], array('query' => array('id' => $batch['id'])));
   $js_setting = array(
     'batch' => array(
-      'errorMessage' => $current_set['error_message'] . '<br/>' . $batch['error_message'],
+      'errorMessage' => $current_set['error_message'] . '<br />' . $batch['error_message'],
       'initMessage' => $current_set['init_message'],
-      'uri' => $url,
+      'uri' => url($batch['url'], array('query' => array('id' => $batch['id']))),
     ),
   );
   drupal_add_js($js_setting, 'setting');
+  drupal_add_js('misc/progress.js', array('cache' => FALSE));
   drupal_add_js('misc/batch.js', array('cache' => FALSE));
 
-  $output = '<div id="progress"></div>';
-  return $output;
+  return '<div id="progress"></div>';
 }
 
 /**
- * Do one pass of execution and inform back the browser about progression
- * (used for JavaScript-mode only).
+ * Do one pass of execution in JavaScript-mode and return progress to the browser.
+ *
+ * @see _batch_progress_page_js()
+ * @see _batch_process()
  */
 function _batch_do() {
-  // HTTP POST required
+  // HTTP POST required.
   if ($_SERVER['REQUEST_METHOD'] != 'POST') {
     drupal_set_message(t('HTTP POST is required.'), 'error');
     drupal_set_title(t('Error'));
@@ -120,36 +139,38 @@ function _batch_do() {
 }
 
 /**
- * Batch processing page without JavaScript support.
+ * Output a batch processing page without JavaScript support.
+ *
+ * @see _batch_process()
  */
 function _batch_progress_page_nojs() {
-  $batch =& batch_get();
-  $current_set = _batch_current_set();
+  $batch = &batch_get();
 
+  $current_set = _batch_current_set();
   drupal_set_title($current_set['title'], PASS_THROUGH);
 
   $new_op = 'do_nojs';
 
   if (!isset($batch['running'])) {
     // This is the first page so we return some output immediately.
-    $percentage = 0;
-    $message = $current_set['init_message'];
+    $percentage       = 0;
+    $message          = $current_set['init_message'];
     $batch['running'] = TRUE;
   }
   else {
-    // This is one of the later requests: do some processing first.
+    // This is one of the later requests; do some processing first.
 
-    // Error handling: if PHP dies due to a fatal error (e.g. non-existant
-    // function), it will output whatever is in the output buffer,
-    // followed by the error message.
+    // Error handling: if PHP dies due to a fatal error (e.g. a nonexistent
+    // function), it will output whatever is in the output buffer, followed by
+    // the error message.
     ob_start();
-    $fallback = $current_set['error_message'] . '<br/>' . $batch['error_message'];
+    $fallback = $current_set['error_message'] . '<br />' . $batch['error_message'];
     $fallback = theme('maintenance_page', $fallback, FALSE, FALSE);
 
     // We strip the end of the page using a marker in the template, so any
-    // additional HTML output by PHP shows up inside the page rather than
-    // below it. While this causes invalid HTML, the same would be true if
-    // we didn't, as content is not allowed to appear after </html> anyway.
+    // additional HTML output by PHP shows up inside the page rather than below
+    // it. While this causes invalid HTML, the same would be true if we didn't,
+    // as content is not allowed to appear after </html> anyway.
     list($fallback) = explode('<!--partial-->', $fallback);
     print $fallback;
 
@@ -159,25 +180,37 @@ function _batch_progress_page_nojs() {
       $new_op = 'finished';
     }
 
-    // PHP did not die : remove the fallback output.
+    // PHP did not die; remove the fallback output.
     ob_end_clean();
   }
 
   $url = url($batch['url'], array('query' => array('id' => $batch['id'], 'op' => $new_op)));
   drupal_set_html_head('<meta http-equiv="Refresh" content="0; URL=' . $url . '">');
-  $output = theme('progress_bar', $percentage, $message);
-  return $output;
+
+  return theme('progress_bar', $percentage, $message);
 }
 
 /**
- * Advance batch processing for 1 second (or process the whole batch if it
- * was not set for progressive execution - e.g forms submitted by drupal_execute).
+ * Process sets in a batch.
+ *
+ * If the batch was marked for progressive execution (default), this executes as
+ * many operations in batch sets until an execution time of 1 second has been
+ * exceeded. It will continue with the next operation of the same batch set in
+ * the next request.
+ *
+ * @return
+ *   An array containing a completion value (in percent) and a status message.
  */
 function _batch_process() {
-  $batch =& batch_get();
-  $current_set =& _batch_current_set();
+  $batch       = &batch_get();
+  $current_set = &_batch_current_set();
+  // Indicate that this batch set needs to be initialized.
   $set_changed = TRUE;
 
+  // If this batch was marked for progressive execution (e.g. forms submitted by
+  // drupal_execute()), initialize a timer to determine whether we need to
+  // proceed with the same batch phase when a processing time of 1 second has
+  // been exceeded.
   if ($batch['progressive']) {
     timer_start('batch_processing');
   }
@@ -191,39 +224,44 @@ function _batch_process() {
     }
 
     $task_message = '';
-    // We assume a single pass operation and set the completion level
-    // to 1 by default.
+    // We assume a single pass operation and set the completion level to 1 by
+    // default.
     $finished = 1;
     if ((list($function, $args) = reset($current_set['operations'])) && function_exists($function)) {
-      // Build the 'context' array, execute the function call,
-      // and retrieve the user message.
-      $batch_context = array('sandbox' => &$current_set['sandbox'], 'results' => &$current_set['results'], 'finished' => &$finished, 'message' => &$task_message);
+      // Build the 'context' array, execute the function call, and retrieve the
+      // user message.
+      $batch_context = array(
+        'sandbox'  => &$current_set['sandbox'],
+        'results'  => &$current_set['results'],
+        'finished' => &$finished,
+        'message'  => &$task_message,
+      );
       // Process the current operation.
       call_user_func_array($function, array_merge($args, array(&$batch_context)));
     }
 
     if ($finished == 1) {
-      // Make sure this step isn't counted double when computing $current.
+      // Make sure this step is not counted twice when computing $current.
       $finished = 0;
-      // Remove the operation and clear the sandbox.
+      // Remove the processed operation and clear the sandbox.
       array_shift($current_set['operations']);
       $current_set['sandbox'] = array();
     }
 
-    // If the batch set is completed, browse through the remaining sets,
-    // executing 'control sets' (stored form submit handlers) along the way -
-    // this might in turn insert new batch sets.
-    // Stop when we find a set that actually has operations.
+    // When all operations in the current batch set are completed, browse
+    // through the remaining sets until we find a set that contains operations.
+    // Note that _batch_next_set() executes stored form submit handlers in
+    // remaining batch sets, which can add new sets to the batch.
     $set_changed = FALSE;
     $old_set = $current_set;
     while (empty($current_set['operations']) && ($current_set['success'] = TRUE) && _batch_next_set()) {
-      $current_set =& _batch_current_set();
+      $current_set = &_batch_current_set();
       $set_changed = TRUE;
     }
-    // At this point, either $current_set is a 'real' batch set (has operations),
-    // or all sets have been completed.
+    // At this point, either $current_set contains operations that need to be
+    // processed or all sets have been completed.
 
-    // If we're in progressive mode, stop after 1 second.
+    // If we are in progressive mode, break processing after 1 second.
     if ($batch['progressive'] && timer_read('batch_processing') > 1000) {
       break;
     }
@@ -234,23 +272,24 @@ function _batch_process() {
 
     // Reporting 100% progress will cause the whole batch to be considered
     // processed. If processing was paused right after moving to a new set,
-    // we have to use the info from the new (unprocessed) one.
+    // we have to use the info from the new (unprocessed) set.
     if ($set_changed && isset($current_set['operations'])) {
       // Processing will continue with a fresh batch set.
-      $remaining = count($current_set['operations']);
-      $total = $current_set['total'];
+      $remaining        = count($current_set['operations']);
+      $total            = $current_set['total'];
       $progress_message = $current_set['init_message'];
-      $task_message = '';
+      $task_message     = '';
     }
     else {
-      $remaining = count($old_set['operations']);
-      $total = $old_set['total'];
+      // Processing will continue with the current batch set.
+      $remaining        = count($old_set['operations']);
+      $total            = $old_set['total'];
       $progress_message = $old_set['progress_message'];
     }
 
     $current    = $total - $remaining + $finished;
     $percentage = $total ? floor($current / $total * 100) : 100;
-    $values = array(
+    $values     = array(
       '@remaining'  => $remaining,
       '@total'      => $total,
       '@current'    => floor($current),
@@ -258,40 +297,46 @@ function _batch_process() {
     );
     $message = strtr($progress_message, $values);
     if (!empty($message)) {
-      $message .= '<br/>';
+      $message .= '<br />';
+    }
+    if (!empty($task_message)) {
+      $message .= $task_message;
     }
-    $message .= $task_message ? $task_message : '&nbsp';
 
     return array($percentage, $message);
   }
   else {
-    // If we're not in progressive mode, the whole batch has been processed by now.
+    // If we are not in progressive mode, the entire batch has been processed.
     return _batch_finished();
   }
-
 }
 
 /**
- * Retrieve the batch set being currently processed.
+ * Return the batch set being currently processed.
  */
 function &_batch_current_set() {
-  $batch =& batch_get();
+  $batch = &batch_get();
   return $batch['sets'][$batch['current_set']];
 }
 
 /**
- * Move execution to the next batch set if any, executing the stored
- * form _submit handlers along the way (thus possibly inserting
- * additional batch sets).
+ * Retrieve the next set in a batch.
+ *
+ * If there is a subsequent set in this batch, assign it as the new set to
+ * process and execute its form submit handler (if defined), which may add
+ * further sets to this batch.
+ *
+ * @return
+ *   TRUE if a subsequent set was found in the batch.
  */
 function _batch_next_set() {
-  $batch =& batch_get();
+  $batch = &batch_get();
   if (isset($batch['sets'][$batch['current_set'] + 1])) {
     $batch['current_set']++;
-    $current_set =& _batch_current_set();
+    $current_set = &_batch_current_set();
     if (isset($current_set['form_submit']) && ($function = $current_set['form_submit']) && function_exists($function)) {
-      // We use our stored copies of $form and $form_state, to account for
-      // possible alteration by the submit handlers.
+      // We use our stored copies of $form and $form_state to account for
+      // possible alterations by previous form submit handlers.
       $function($batch['form'], $batch['form_state']);
     }
     return TRUE;
@@ -299,17 +344,18 @@ function _batch_next_set() {
 }
 
 /**
- * End the batch processing:
- * Call the 'finished' callbacks to allow custom handling of results,
- * and resolve page redirection.
+ * End the batch processing.
+ *
+ * Call the 'finished' callback of each batch set to allow custom handling of
+ * the results and resolve page redirection.
  */
 function _batch_finished() {
-  $batch =& batch_get();
+  $batch = &batch_get();
 
-  // Execute the 'finished' callbacks for each batch set.
+  // Execute the 'finished' callbacks for each batch set, if defined.
   foreach ($batch['sets'] as $key => $batch_set) {
     if (isset($batch_set['finished'])) {
-      // Check if the set requires an additional file for functions definitions.
+      // Check if the set requires an additional file for function definitions.
       if (isset($batch_set['file']) && is_file($batch_set['file'])) {
         include_once DRUPAL_ROOT . '/' . $batch_set['file'];
       }
@@ -319,7 +365,7 @@ function _batch_finished() {
     }
   }
 
-  // Cleanup the batch table and unset the global $batch variable.
+  // Clean up the batch table and unset the static $batch variable.
   if ($batch['progressive']) {
     db_delete('batch')
       ->condition('bid', $batch['id'])
@@ -330,13 +376,12 @@ function _batch_finished() {
 
   // Redirect if needed.
   if ($_batch['progressive']) {
-    // Put back the 'destination' that was saved in batch_process().
+    // Revert the 'destination' that was saved in batch_process().
     if (isset($_batch['destination'])) {
       $_REQUEST['destination'] = $_batch['destination'];
     }
 
-    // Use $_batch['form_state']['redirect'], or $_batch['redirect'],
-    // or $_batch['source_page'].
+    // Determine the target path to redirect to.
     if (isset($_batch['form_state']['redirect'])) {
       $redirect = $_batch['form_state']['redirect'];
     }
@@ -347,7 +392,7 @@ function _batch_finished() {
       $redirect = $_batch['source_page'];
     }
 
-    // Let drupal_redirect_form handle redirection logic.
+    // Use drupal_redirect_form() to handle the redirection logic.
     $form = isset($batch['form']) ? $batch['form'] : array();
     if (empty($_batch['form_state']['rebuild']) && empty($_batch['form_state']['storage'])) {
       drupal_redirect_form($form, $redirect);
@@ -355,15 +400,14 @@ function _batch_finished() {
 
     // We get here if $form['#redirect'] was FALSE, or if the form is a
     // multi-step form. We save the final $form_state value to be retrieved
-    // by drupal_get_form, and we redirect to the originating page.
+    // by drupal_get_form(), and redirect to the originating page.
     $_SESSION['batch_form_state'] = $_batch['form_state'];
     drupal_goto($_batch['source_page']);
   }
 }
 
 /**
- * Shutdown function: store the batch data for next request,
- * or clear the table if the batch is finished.
+ * Shutdown function; store the current batch data for the next request.
  */
 function _batch_shutdown() {
   if ($batch = batch_get()) {
@@ -373,3 +417,4 @@ function _batch_shutdown() {
       ->execute();
   }
 }
+
diff --git a/includes/form.inc b/includes/form.inc
index daa7bd8ae..b9b2ebf3f 100644
--- a/includes/form.inc
+++ b/includes/form.inc
@@ -2497,7 +2497,7 @@ function form_clean_id($id = NULL, $flush = FALSE) {
  *        @endcode
  *     All the other values below are optional.
  *     batch_init() provides default values for the messages.
- *     'title': title for the progress page.
+ *     'title': title for the progress page. Only safe strings should be passed.
  *       Defaults to t('Processing').
  *     'init_message': message displayed while the processing is initialized.
  *       Defaults to t('Initializing.').