#353494 by Damien Tournoud: make module_invoke() and module_invoke_all() accept passed-by-reference arguments, and remove node_invoke(), node_invoke_nodeapi() and comment_invoke_comment().

From: Damien Tournoud <damien@tournoud.net>


---

 includes/module.inc                    |   28 ++++----
 modules/blogapi/blogapi.module         |    4 +
 modules/comment/comment.admin.inc      |    4 +
 modules/comment/comment.module         |   36 +----------
 modules/node/node.module               |  107 ++++++++++----------------------
 modules/node/node.pages.inc            |   15 +++-
 modules/search/search.api.php          |   10 +--
 modules/translation/translation.module |    2 -
 8 files changed, 71 insertions(+), 135 deletions(-)


diff --git includes/module.inc includes/module.inc
index c0566d8..155cc01 100644
--- includes/module.inc
+++ includes/module.inc
@@ -489,17 +489,17 @@ function _module_implements_maintenance($hook, $sort = FALSE) {
  * @param $hook
  *   The name of the hook to invoke.
  * @param ...
- *   Arguments to pass to the hook implementation.
+ *   Arguments (max 10) to pass to the hook implementation. Arguments can be
+ *   passed by reference by prefixing them with "&".
  * @return
  *   The return value of the hook implementation.
  */
-function module_invoke() {
-  $args = func_get_args();
-  $module = $args[0];
-  $hook = $args[1];
-  unset($args[0], $args[1]);
-  if (module_hook($module, $hook)) {
-    return call_user_func_array($module . '_' . $hook, $args);
+function module_invoke($module, $hook, $a1 = NULL, $a2 = NULL, $a3 = NULL, $a4 = NULL, $a5 = NULL, $a6 = NULL, $a7 = NULL, $a8 = NULL, $a9 = NULL, $a10 = NULL) {
+  $function = $module . '_' . $hook;
+  if (drupal_function_exists($function)) {
+    // We don't use func_get_args() / call_user_func_array() here because it's
+    // slower and doesn't allow arguments to be passed by reference.
+    return $function($a1, $a2, $a3, $a4, $a5, $a6, $a7, $a8, $a9, $a10);
   }
 }
 /**
@@ -508,20 +508,20 @@ function module_invoke() {
  * @param $hook
  *   The name of the hook to invoke.
  * @param ...
- *   Arguments to pass to the hook.
+ *   Arguments (max 10) to pass to the hook implementation. Arguments can be
+ *   passed by reference by prefixing them with "&".
  * @return
  *   An array of return values of the hook implementations. If modules return
  *   arrays from their implementations, those are merged into one array.
  */
-function module_invoke_all() {
-  $args = func_get_args();
-  $hook = $args[0];
-  unset($args[0]);
+function module_invoke_all($hook, $a1 = NULL, $a2 = NULL, $a3 = NULL, $a4 = NULL, $a5 = NULL, $a6 = NULL, $a7 = NULL, $a8 = NULL, $a9 = NULL, $a10 = NULL) {
   $return = array();
   foreach (module_implements($hook) as $module) {
     $function = $module . '_' . $hook;
     if (drupal_function_exists($function)) {
-      $result = call_user_func_array($function, $args);
+      // We don't use func_get_args() / call_user_func_array() here because it's
+      // slower and doesn't allow arguments to be passed by reference.
+      $result = $function($a1, $a2, $a3, $a4, $a5, $a6, $a7, $a8, $a9, $a10);
       if (isset($result) && is_array($result)) {
         $return = array_merge_recursive($return, $result);
       }
diff --git modules/blogapi/blogapi.module modules/blogapi/blogapi.module
index 792b50d..fb75293 100644
--- modules/blogapi/blogapi.module
+++ modules/blogapi/blogapi.module
@@ -224,7 +224,7 @@ function blogapi_blogger_new_post($appkey, $blogid, $username, $password, $conte
     $edit['date'] = format_date(REQUEST_TIME, 'custom', 'Y-m-d H:i:s O');
   }
 
-  node_invoke_nodeapi($edit, 'blogapi_new');
+  module_invoke_all('nodeapi_blogapi_new', &$edit);
 
   $valid = blogapi_status_error_check($edit, $publish);
   if ($valid !== TRUE) {
@@ -282,7 +282,7 @@ function blogapi_blogger_edit_post($appkey, $postid, $username, $password, $cont
     $node->body = $content;
   }
 
-  node_invoke_nodeapi($node, 'blogapi_edit');
+  module_invoke_all('nodeapi_blogapi_edit', &$node);
 
   $valid = blogapi_status_error_check($node, $original_status);
   if ($valid !== TRUE) {
diff --git modules/comment/comment.admin.inc modules/comment/comment.admin.inc
index 9c6083a..9435f60 100644
--- modules/comment/comment.admin.inc
+++ modules/comment/comment.admin.inc
@@ -134,7 +134,7 @@ function comment_admin_overview_submit($form, &$form_state) {
         $comment = comment_load($cid);
         _comment_update_node_statistics($comment->nid);
         // Allow modules to respond to the updating of a comment.
-        comment_invoke_comment($comment, $form_state['values']['operation']);
+        module_invoke_all('comment', &$comment, $form_state['values']['operation']);
         // Add an entry to the watchdog log.
         watchdog('content', 'Comment: updated %subject.', array('%subject' => $comment->subject), WATCHDOG_NOTICE, l(t('view'), 'node/' . $comment->nid, array('fragment' => 'comment-' . $comment->cid)));
       }
@@ -310,7 +310,7 @@ function _comment_delete_thread($comment) {
   // Delete the comment.
   db_query('DELETE FROM {comment} WHERE cid = %d', $comment->cid);
   watchdog('content', 'Comment: deleted %subject.', array('%subject' => $comment->subject));
-  comment_invoke_comment($comment, 'delete');
+  module_invoke_all('comment', &$comment, 'delete');
 
   // Delete the comment's replies.
   $result = db_query('SELECT c.*, u.name AS registered_name, u.uid FROM {comment} c INNER JOIN {users} u ON u.uid = c.uid WHERE pid = %d', $comment->cid);
diff --git modules/comment/comment.module modules/comment/comment.module
index 7e13855..26a3115 100644
--- modules/comment/comment.module
+++ modules/comment/comment.module
@@ -781,7 +781,7 @@ function comment_save($edit) {
           ->condition('cid', $edit['cid'])
           ->execute();
         // Allow modules to respond to the updating of a comment.
-        comment_invoke_comment($edit, 'update');
+        module_invoke_all('comment', &$edit, 'update');
         // Add an entry to the watchdog log.
         watchdog('content', 'Comment: updated %subject.', array('%subject' => $edit['subject']), WATCHDOG_NOTICE, l(t('view'), 'node/' . $edit['nid'], array('fragment' => 'comment-' . $edit['cid'])));
       }
@@ -853,7 +853,7 @@ function comment_save($edit) {
           ))
           ->execute();
         // Tell the other modules a new comment has been submitted.
-        comment_invoke_comment($edit, 'insert');
+        module_invoke_all('comment', &$edit, 'insert');
         // Add an entry to the watchdog log.
         watchdog('content', 'Comment: added %subject.', array('%subject' => $edit['subject']), WATCHDOG_NOTICE, l(t('view'), 'node/' . $edit['nid'], array('fragment' => 'comment-' . $edit['cid'])));
       }
@@ -870,7 +870,7 @@ function comment_save($edit) {
       }
       else {
         drupal_set_message(t('Your comment has been posted.'));
-        comment_invoke_comment($edit, 'publish');
+        module_invoke_all('comment', &$edit, 'publish');
       }
 
       return $edit['cid'];
@@ -1259,7 +1259,7 @@ function comment_validate($edit) {
   global $user;
 
   // Invoke other validation handlers.
-  comment_invoke_comment($edit, 'validate');
+  module_invoke_all('comment', &$edit, 'validate');
 
   if (isset($edit['date'])) {
     if (strtotime($edit['date']) === FALSE) {
@@ -1735,7 +1735,7 @@ function theme_comment_view($comment, $node, $links = array(), $visible = TRUE)
   if ($visible) {
     $comment->comment = check_markup($comment->comment, $comment->format, '', FALSE);
     // Comment API hook.
-    comment_invoke_comment($comment, 'view');
+    module_invoke_all('comment', &$comment, 'view');
     $output .= theme('comment', $comment, $node, $links);
   }
   else {
@@ -1990,32 +1990,6 @@ function _comment_update_node_statistics($nid) {
 }
 
 /**
- * Invoke a hook_comment() operation in all modules.
- *
- * @param &$comment
- *   A comment object.
- * @param $op
- *   A string containing the name of the comment operation.
- * @return
- *   The returned value of the invoked hooks.
- */
-function comment_invoke_comment(&$comment, $op) {
-  $return = array();
-  foreach (module_implements('comment') as $module) {
-    $function = $module . '_comment';
-    $result = $function($comment, $op);
-    if (isset($result) && is_array($result)) {
-      $return = array_merge($return, $result);
-    }
-    elseif (isset($result)) {
-      $return[] = $result;
-    }
-  }
-
-  return $return;
-}
-
-/**
  * Generate vancode.
  *
  * Consists of a leading character indicating length, followed by N digits
diff --git modules/node/node.module modules/node/node.module
index f561972..9b8e1b5 100644
--- modules/node/node.module
+++ modules/node/node.module
@@ -670,66 +670,20 @@ function node_type_set_defaults($info = array()) {
 }
 
 /**
- * Determine whether a node hook exists.
+ * Return the name of a node function, if it exists.
  *
  * @param &$node
  *   Either a node object, node array, or a string containing the node type.
  * @param $hook
  *   A string containing the name of the hook.
  * @return
- *   TRUE iff the $hook exists in the node type of $node.
+ *   The name of the function implementing the hook for $node, or NULL.
  */
 function node_hook(&$node, $hook) {
-  $base = node_get_types('base', $node);
-  return module_hook($base, $hook);
-}
-
-/**
- * Invoke a node hook.
- *
- * @param &$node
- *   Either a node object, node array, or a string containing the node type.
- * @param $hook
- *   A string containing the name of the hook.
- * @param $a2, $a3, $a4
- *   Arguments to pass on to the hook, after the $node argument.
- * @return
- *   The returned value of the invoked hook.
- */
-function node_invoke(&$node, $hook, $a2 = NULL, $a3 = NULL, $a4 = NULL) {
-  if (node_hook($node, $hook)) {
-    $base = node_get_types('base', $node);
-    $function = $base . '_' . $hook;
-    return ($function($node, $a2, $a3, $a4));
-  }
-}
-
-/**
- * Invoke a hook_nodeapi() operation in all modules.
- *
- * @param &$node
- *   A node object.
- * @param $op
- *   A string containing the name of the nodeapi operation.
- * @param $a3, $a4
- *   Arguments to pass on to the hook, after the $node and $op arguments.
- * @return
- *   The returned value of the invoked hooks.
- */
-function node_invoke_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) {
-  $return = array();
-  $hook = 'nodeapi_' . $op;
-  foreach (module_implements($hook) as $module) {
-    $function = $module . '_' . $hook;
-    $result = $function($node, $a3, $a4);
-    if (isset($result) && is_array($result)) {
-      $return = array_merge($return, $result);
-    }
-    elseif (isset($result)) {
-      $return[] = $result;
-    }
+  $function = node_get_types('base', $node) . '_' . $hook;
+  if (drupal_function_exists($function)) {
+    return $function;
   }
-  return $return;
 }
 
 /**
@@ -940,8 +894,10 @@ function node_validate($node, $form = array()) {
   }
 
   // Do node-type-specific validation checks.
-  node_invoke($node, 'validate', $form);
-  node_invoke_nodeapi($node, 'validate', $form);
+  if ($function = node_hook($node, 'validate')) {
+    $function(&$node, $form);
+  }
+  module_invoke_all('nodeapi_validate', &$node, $form);
 }
 
 /**
@@ -991,7 +947,7 @@ function node_submit($node) {
  */
 function node_save(&$node) {
   // Let modules modify the node before it is saved to the database.
-  node_invoke_nodeapi($node, 'presave');
+  module_invoke_all('nodeapi_presave', &$node);
   global $user;
 
   $node->is_new = FALSE;
@@ -1061,11 +1017,11 @@ function node_save(&$node) {
     db_query('UPDATE {node} SET vid = %d WHERE nid = %d', $node->vid, $node->nid);
   }
 
-  // Call the node specific callback (if any). This can be
-  // node_invoke($node, 'insert') or
-  // node_invoke($node, 'update').
-  node_invoke($node, $op);
-  node_invoke_nodeapi($node, $op);
+  // Call the node specific callback (if any).
+  if ($function = node_hook($node, $op)) {
+    $function(&$node);
+  }
+  module_invoke_all('nodeapi_' . $op, &$node);
 
   // Update the node access table for this node.
   node_access_acquire_grants($node);
@@ -1103,9 +1059,11 @@ function node_delete($nid) {
     db_query('DELETE FROM {node} WHERE nid = %d', $node->nid);
     db_query('DELETE FROM {node_revision} WHERE nid = %d', $node->nid);
 
-    // Call the node-specific callback (if any):
-    node_invoke($node, 'delete');
-    node_invoke_nodeapi($node, 'delete');
+    // Call the node-specific callback (if any).
+    if ($function = node_hook($node, 'delete')) {
+      $function(&$node);
+    }
+    module_invoke_all('nodeapi_delete', &$node);
 
     // Clear the page and block caches.
     cache_clear_all();
@@ -1191,15 +1149,15 @@ function node_build_content($node, $teaser = FALSE) {
 
   // The 'view' hook can be implemented to overwrite the default function
   // to display nodes.
-  if (node_hook($node, 'view')) {
-    $node = node_invoke($node, 'view', $teaser);
+  if ($function = node_hook($node, 'view')) {
+    $node = $function(&$node, $teaser);
   }
   else {
     $node = node_prepare($node, $teaser);
   }
 
   // Allow modules to make their own additions to the node.
-  node_invoke_nodeapi($node, 'view', $teaser);
+  module_invoke_all('nodeapi_view', &$node, $teaser);
   
   // Allow modules to modify the structured node.
   drupal_alter('node_view', $node, $teaser);
@@ -1422,7 +1380,7 @@ function node_search($op = 'search', $keys = NULL) {
         // Fetch terms for snippet.
         $node->body .= module_invoke('taxonomy', 'nodeapi', $node, 'update_index');
 
-        $extra = node_invoke_nodeapi($node, 'search_result');
+        $extra = module_invoke_all('nodeapi_search_result', &$node);
 
         $results[] = array(
           'link' => url('node/' . $item->sid, array('absolute' => TRUE)),
@@ -1814,19 +1772,19 @@ function node_feed($nids = FALSE, $channel = array()) {
       $teaser = ($item_length == 'teaser');
 
       // Filter and prepare node teaser
-      if (node_hook($item, 'view')) {
-        $item = node_invoke($item, 'view', $teaser, FALSE);
+      if ($function = node_hook($item, 'view')) {
+        $item = $function(&$item, $teaser, FALSE);
       }
       else {
         $item = node_prepare($item, $teaser);
       }
 
       // Allow modules to change $node->teaser before viewing.
-      node_invoke_nodeapi($item, 'view', $teaser, FALSE);
+      module_invoke_all('nodeapi_view', &$item, $teaser, FALSE);
     }
 
     // Allow modules to add additional item fields and/or modify $item
-    $extra = node_invoke_nodeapi($item, 'rss_item');
+    $extra = module_invoke_all('nodeapi_rss_item', &$item);
     $extra = array_merge($extra, array(array('key' => 'pubDate', 'value' => gmdate('r', $item->created)), array('key' => 'dc:creator', 'value' => $item->name), array('key' => 'guid', 'value' => $item->nid . ' at ' . $base_url, 'attributes' => array('isPermaLink' => 'false'))));
     foreach ($extra as $element) {
       if (isset($element['namespace'])) {
@@ -1948,7 +1906,7 @@ function _node_index_node($node) {
   $text = '<h1>' . check_plain($node->title) . '</h1>' . $node->body;
 
   // Fetch extra data normally not visible
-  $extra = node_invoke_nodeapi($node, 'update_index');
+  $extra = module_invoke_all('nodeapi_update_index', &$node);
   foreach ($extra as $t) {
     $text .= $t;
   }
@@ -2162,10 +2120,9 @@ function node_access($op, $node, $account = NULL) {
     return FALSE;
   }
 
-  // Can't use node_invoke('access', $node), because the access hook takes the
-  // $op parameter before the $node parameter.
-  $base = node_get_types('base', $node);
-  $access = module_invoke($base, 'access', $op, $node, $account);
+  if ($function = node_hook($node, 'access')) {
+    $access = $function($op, $node, $account);
+  }
   if (!is_null($access)) {
     return $access;
   }
diff --git modules/node/node.pages.inc modules/node/node.pages.inc
index 97bfb5e..8e38140 100644
--- modules/node/node.pages.inc
+++ modules/node/node.pages.inc
@@ -88,8 +88,10 @@ function node_object_prepare(&$node) {
   // Always use the default revision setting.
   $node->revision = in_array('revision', $node_options);
 
-  node_invoke($node, 'prepare');
-  node_invoke_nodeapi($node, 'prepare');
+  if ($function = node_hook($node, 'prepare')) {
+    $function(&$node);
+  }
+  module_invoke_all('nodeapi_prepare', &$node);
 }
 
 /**
@@ -136,8 +138,11 @@ function node_form(&$form_state, $node) {
     '#default_value' => isset($node->changed) ? $node->changed : NULL,
   );
   // Get the node-specific bits.
-  if ($extra = node_invoke($node, 'form', $form_state)) {
-    $form = array_merge_recursive($form, $extra);
+  if ($function = node_hook($node, 'form')) {
+    $extra = $function(&$node, $form_state);
+    if (is_array($extra)) {
+      $form = array_merge_recursive($form, $extra);
+    }
   }
   if (!isset($form['title']['#weight'])) {
     $form['title']['#weight'] = -5;
@@ -576,7 +581,7 @@ function node_revision_delete_confirm($form_state, $node_revision) {
 function node_revision_delete_confirm_submit($form, &$form_state) {
   $node_revision = $form['#node_revision'];
   db_query("DELETE FROM {node_revision} WHERE nid = %d AND vid = %d", $node_revision->nid, $node_revision->vid);
-  node_invoke_nodeapi($node_revision, 'delete_revision');
+  module_invoke_all('nodeapi_delete_revision', &$node_revision);
   watchdog('content', '@type: deleted %title revision %revision.', array('@type' => $node_revision->type, '%title' => $node_revision->title, '%revision' => $node_revision->vid));
   drupal_set_message(t('Revision from %revision-date of @type %title has been deleted.', array('%revision-date' => format_date($node_revision->revision_timestamp), '@type' => node_get_types('name', $node_revision), '%title' => $node_revision->title)));
   $form_state['redirect'] = 'node/' . $node_revision->nid;
diff --git modules/search/search.api.php modules/search/search.api.php
index 5573668..2963e39 100644
--- modules/search/search.api.php
+++ modules/search/search.api.php
@@ -177,7 +177,7 @@ function hook_search($op = 'search', $keys = null) {
         // Fetch terms for snippet.
         $node->body .= module_invoke('taxonomy', 'nodeapi', $node, 'update_index');
 
-        $extra = node_invoke_nodeapi($node, 'search_result');
+        $extra = module_invoke_all('nodeapi_search_result', &$node);
 
         $results[] = array(
           'link' => url('node/' . $item->sid, array('absolute' => TRUE)),
@@ -258,19 +258,19 @@ function hook_update_index() {
     variable_set('node_cron_last', max($last_comment, $node->changed, $node->created));
 
     // Get node output (filtered and with module-specific fields).
-    if (node_hook($node, 'view')) {
-      node_invoke($node, 'view', false, false);
+    if ($function = node_hook($node, 'view')) {
+      $function(&$node, false, false);
     }
     else {
       $node = node_prepare($node, false);
     }
     // Allow modules to change $node->body before viewing.
-    node_invoke_nodeapi($node, 'view', false, false);
+    module_invoke_all('nodeapi_view', &$node, false, false);
 
     $text = '<h1>' . drupal_specialchars($node->title) . '</h1>' . $node->body;
 
     // Fetch extra data normally not visible
-    $extra = node_invoke_nodeapi($node, 'update_index');
+    $extra = module_invoke_all('nodeapi_update_index', &$node);
     foreach ($extra as $t) {
       $text .= $t;
     }
diff --git modules/translation/translation.module modules/translation/translation.module
index cb5df9e..116b0ab 100644
--- modules/translation/translation.module
+++ modules/translation/translation.module
@@ -201,7 +201,7 @@ function translation_nodeapi_prepare($node) {
       $node->title = $node->translation_source->title;
       $node->body = $node->translation_source->body;
       // Let every module add custom translated fields.
-      node_invoke_nodeapi($node, 'prepare_translation');
+      module_invoke_all('nodeapi_prepare_translation', &$node);
     }
   }
 }
