I'm not 100% sure this is a bug, but I certainly found it surprising. Trying running the following script on a Drupal 7 site that has at least one preexisting node:
define('DRUPAL_ROOT', getcwd());
require_once DRUPAL_ROOT . '/includes/bootstrap.inc';
drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);
// Load an existing node on the site and print its title.
$node = node_load(1);
print 'Original title: ' . $node->title . '<br />';
// Change the title but do NOT save the modified node.
$node->title = 'new title';
// Reload the node and print its title. Even though the node was never
// saved, it will still show the new title!
$reloaded_node = node_load(1);
print 'New title: ' . $reloaded_node->title;
It does exactly what the code comments say. (In Drupal 6, it prints the original node title both times, like you might expect).
I think this is a subtle issue that has to do with the way PHP5 handles object references, or something like that... Bug or not?
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | issue-424602-2.patch | 834 bytes | David_Rothstein |
Comments
Comment #1
David_Rothstein commentedThis bug/feature appears to be the reason that #221081: Entity cache out of sync and inconsistent when saving/deleting entities is not an issue in Drupal 7 (in fact, I discovered this while trying to fix that bug).
I'm still pretty sure it's unintentional, though, since the correct way to fix that bug is to make sure the internal node_load() static cache gets cleared, and that is not currently happening in Drupal 7.
Comment #2
David_Rothstein commentedThis seems to fix it, but I'm not 100% sure it's the correct solution.
It could probably use a test too, but no use writing one until confirming that it is in fact considered a bug.
Comment #3
David_Rothstein commentedI'm guessing other functions in Drupal are affected by this too. For example, this function appears to work similarly: http://api.drupal.org/api/function/taxonomy_vocabulary_load_multiple/7
Comment #4
swentel commentedI can also reproduce this on my local install. This is a bug imo, although I can't think of a direct use case where I wouldn't call a node_save if I change the value of a key in a node object ..
Comment #6
David_Rothstein commentedShould be "needs work", since there's a patch. Also, just today there's been some parallel work on a patch at #221081: Entity cache out of sync and inconsistent when saving/deleting entities that may already have a test, but I haven't looked at it closely.
Comment #7
schwern commentedIn the process of analyzing another bug I came across this one. Analysis and test here:
http://drupal.org/node/221081#comment-1539392
Conclusion: Yes, it is a regression. Yes it is bad. It exposes the existence of the node cache and causes action at a distance.
Consider if module A loads node 1, makes some changes to the node and then calls module B which calls C which calls D which calls E which loads node 1. It will now see module A's unsaved and possibly incomplete changes to node 1. This is almost impossible to debug.
A solution is to go back to what 6 did, copy before caching *AND* before returning. The patch in #2 only copies on cache. Though that will then break the bug reported in http://drupal.org/node/221081 which which change incidentally fixed. It needs to be fixed in a different way.
Comment #8
catchThere's a patch here. I'm also fairly certain there's an old issue which found dozens of instances like this although not sure where to find it.
Comment #9
catchHere it is #154859: Document that cached entity objects are not necessarily cloned any more