[development] Drupal 7 node_load and node_save caching behavior bug

[David Goode]

Hello fellow developers,

In fixing an error related to the notifications module in Drupal 5 and 6, I
came across an unexpected behavior that merits discussion, and possibly a
fix in Drupal 7, as the functionality doesn't appear to have changed. The
immediate issue is on pages such as the node edit form when using the
notifications module with immediate sending, but applies in many other
places. It occurs when these actions occur sequentially, which is quite
commonplace:

1: a node_load is performed for [nid]
2: a node_save is performed on the same [nid], updating the database
accordingly
3: either during the nodeapi('update') hook or afterwards, a node_load is
performed on [nid]

When number 3 occurs, it will load the OLD node, prior to the update,
because the node will have been CACHED in the static variable of the
node_load function.

------------------------------
-----------------------------------------------

I believe this generally hasn't been found to be a problem because the new
$node is passed around through all the update hooks, so node_loads aren't
generally required. However, I came across the problem in fixing an issue
for the notifications module (http://drupal.org/node/278530). What happens
is notifications_content_nodeapi adds an event to the queue, but the $node
object is not passed around. If notifications is set to send immediately, it
will call the sending stack, which does a node_load on the node and creates
the message. However, it was sending the old message when an update event
occurred (because it had been cached), causing a user to report the issue.

For notifications I have fixed this issue by simply node_loading and telling
it to clear the cache in notifications_content_nodeapi. However, I wonder
whether a better default behavior would be to have node_save clear the cache
itself?

I don't know of modules that depend on either 1) showing the old node
throughout the page after a node_save or 2) showing the same version of the
node in two places, between which a node_save is performed. However, if
there are any they might be affected by this change. Either way, I believe
it merits discussion. Also, if it is to be performed, it is a very simple
fix:

node_load(0, NULL, TRUE);

but I am not very familiar with the rationale behind the node_save,
node_save_revision, drupal_write_record etc functions in drupal 7, so
perhaps there is a better place to put it than just in the node_save
function somewhere. It could even be in an optional module that is called on
nodeapi('update') with a very low weight.

David Goode
davidgoode at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.drupal.org/pipermail/development/attachments/20080711/869eaf4e/attachment.htm