Drupal 7 node_load and node_save caching behavior bug
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@gmail.com
At first blush I think it is correct that node_save should flush the node_load cache. I was bitten by this behavior recently as well.
However, I wonder whether a better default behavior would be to have node_save clear the cache itself?
However, I wonder whether a better default behavior would be to have node_save clear the cache itself?
This issue has been around for a long time, and is linked to the way Drupal use static variables for caching: http://drupal.org/node/221081 for Drupal 5 http://drupal.org/node/111127 a proposal to cache node_load() The best answer so far is the 'static cache facility': http://drupal.org/node/254491 the static cache facility Damien Tournoud http://drupal.org/user/22211
participants (3)
-
Damien -
David Goode -
Robert Douglass