Update hook_nodeapi_* functions signatures and documentation

So in #306224: EOL Taxonomy sprint: add proper taxonomy term hooks I'm just using module_invoke_all('taxonomy_term_$op', $term) for each of the different hooks - since objects are now passed by reference in PHP5. So if we can ensure that every hook just takes a term object, then we could completely remove node_invoke_nodeapi() too.

Here is a patch.

Besides making function signatures consistent by removing non-existent arguments, it also make hook_nodeapi_load() implementations modify node object directly, instead of returning an array of values to be appended to node object.

@recidive: please remove the nodeapi_load stuff from your patch, as this hook as already been modified by the RTBC #324313: Load multiple nodes and terms at once.

#324313: Load multiple nodes and terms at once is not yet commited.

I'm ok re-rolling this one when the other goes in.

I'm just trying to prevent duplicated work.

Removed nodeapi_load() and node_load() changes from patch as per request on #3, keeping only the signatures updates (sorry @Damien Tournoud, I thought you meant removing the signature updates).

The patch is now strictly focused on making hook signatures consistent.

Here is a list of where/how hooks are called for reference:

modules/blogapi/blogapi.module:227:  node_invoke_nodeapi($edit, 'blogapi_new')
modules/blogapi/blogapi.module:285:  node_invoke_nodeapi($node, 'blogapi_edit')
modules/translation/translation.module:202: node_invoke_nodeapi($node, 'prepare_translation')
modules/node/node.module:803:  node_invoke($node, 'load')
modules/node/node.module:809:  node_invoke_nodeapi($node, 'load')
modules/node/node.module:856:  node_invoke($node, 'validate', $form)
modules/node/node.module:857:  node_invoke_nodeapi($node, 'validate', $form)
modules/node/node.module:907:  node_invoke_nodeapi($node, 'presave')
modules/node/node.module:980:  node_invoke($node, 'insert') or
modules/node/node.module:980:  node_invoke($node, 'update')
modules/node/node.module:981:  node_invoke_nodeapi($node, 'insert') or
modules/node/node.module:981:  node_invoke_nodeapi($node, 'update')
modules/node/node.module:1020: node_invoke($node, 'delete')
modules/node/node.module:1021: node_invoke_nodeapi($node, 'delete')
modules/node/node.module:1076: node_invoke_nodeapi($node, 'alter', $teaser, $page)
modules/node/node.module:1132: node_invoke($node, 'view', $teaser, $page)
modules/node/node.module:1139: node_invoke_nodeapi($node, 'view', $teaser, $page)
modules/node/node.module:1362: node_invoke_nodeapi($node, 'search_result')
modules/node/node.module:1765: node_invoke($item, 'view', $teaser, FALSE)
modules/node/node.module:1772: node_invoke_nodeapi($item, 'view', $teaser, FALSE)
modules/node/node.module:1776: node_invoke_nodeapi($item, 'rss_item')
modules/node/node.module:1902: node_invoke_nodeapi($node, 'update_index')
modules/node/node.pages.inc:91:  node_invoke($node, 'prepare')
modules/node/node.pages.inc:92:  node_invoke_nodeapi($node, 'prepare')
modules/node/node.pages.inc:139: node_invoke($node, 'form', $form_state)
modules/node/node.pages.inc:579: node_invoke_nodeapi($node_revision, 'delete_revision')

Moving to Drupal core since node.api.php is now a part of core: http://drupal.org/node/314870

Whoops, moved the wrong issue. This issue is a duplicate of http://drupal.org/node/314244 .

This is not a duplicate of #314244: remove $op from hook_nodeapi.

This issue is about code: making nodeapi function signatures consistent.

Fixing taxonomy issues that caused tests to fail.

Should we merge this and the documentation issue? Seems like essentialy the same cleanup and it's possible now with modul.api.php - would make it easier to review on both ends I think.

@catch: agreed.

I've added documentation to the patch. And closed the other issue.

Re-rolled.

Weird, testbot already tested the latest patch and all tests passed. But the result didn't get published here.

See http://testing.drupal.org/pifr/file/1/2196

This might be out of scope for this issue, but looking at the patch, I see a lot of one line function calls:

 /**
  * Implementation of hook_nodeapi_delete().
  */
-function taxonomy_nodeapi_delete($node, $arg = 0) {
+function taxonomy_nodeapi_delete($node) {
   taxonomy_node_delete($node);
 }
 
 /**
  * Implementation of hook_nodeapi_delete_revision().
  */
-function taxonomy_nodeapi_delete_revision($node, $arg = 0) {
+function taxonomy_nodeapi_delete_revision($node) {
   taxonomy_node_delete_revision($node);
 }
 
 /**
  * Implementation of hook_nodeapi_validate().
  */
-function taxonomy_nodeapi_validate($node, $arg = 0) {
+function taxonomy_nodeapi_validate($node, $form) {
   taxonomy_node_validate($node);
 }
 
 /**
  * Implementation of hook_nodeapi_rss_item().
  */
-function taxonomy_nodeapi_rss_item($node, $arg = 0) {
+function taxonomy_nodeapi_rss_item($node) {
   return taxonomy_rss_item($node);
 }
 
 /**
  * Implementation of hook_nodeapi_update_index().
  */
-function taxonomy_nodeapi_update_index($node, $arg = 0) {
+function taxonomy_nodeapi_update_index($node) {
   return taxonomy_node_update_index($node);
 }

Instead of having these one line function calls, could we just remove those functions and then rename their corresponding function calls to the proper nodeapi hook? For example, remove taxonomy_nodeapi_delete, and rename taxonomy_node_delete to taxonomy_nodeapi_delete. Remove taxonomy_nodeapi_validate, rename taxonomy_node_validate to taxonomy_nodeapi_validate....

@Rob Loach : yes. but I think this is out of the scope of this issue. It would be better if it was done on "the get rid of $op from hook_nodeapi" one.

We should submit a new issue to remove those indirect calls.

It's not a -1 for your suggestion. I'm just trying to keep this patch as simple as possible, so it get in faster.

Anyway, thanks for the review!

Tests pass, I read through the patch and it looks good.
We should sort out taxonomy_nodeapi_*, and also trigger.module needs similar treatment - but these can be followup patches. Nice work!

Great! Once this is in, we can hack at #344361: Remove cruft left by de-op nodeapi patch

Committed to CVS HEAD. Thanks! I almost forgot about this clean-up ... :)