diff --git includes/common.inc includes/common.inc index 42b6395..3a95a6b 100644 --- includes/common.inc +++ includes/common.inc @@ -7320,6 +7320,28 @@ function entity_load($entity_type, $ids = FALSE, $conditions = array(), $reset = } /** + * Loads the unchanged, i.e. not modified, entity from the database. + * + * Unlike entity_load() this function ensures the entity is directly loaded from + * the database, thus bypassing any static cache. In particular, this function + * is useful to determine changes by comparing the entity being saved to the + * stored entity. + * + * @param $entity_type + * The entity type to load, e.g. node or user. + * @param $id + * The id of the entity to load. + * + * @return + * The unchanged entity, or FALSE if the entity cannot be loaded. + */ +function entity_load_unchanged($entity_type, $id) { + entity_get_controller($entity_type)->resetCache(array($id)); + $result = entity_get_controller($entity_type)->load(array($id)); + return reset($result); +} + +/** * Get the entity controller class for an entity type. */ function entity_get_controller($entity_type) { diff --git includes/entity.inc includes/entity.inc index 1c3e749..5fb36e2 100644 --- includes/entity.inc +++ includes/entity.inc @@ -24,8 +24,12 @@ interface DrupalEntityControllerInterface { /** * Resets the internal, static entity cache. + * + * @param $ids + * (optional) If specified, the cache is reset for the entities with the + * given ids only. */ - public function resetCache(); + public function resetCache(array $ids = NULL); /** * Loads one or more entities. @@ -139,8 +143,15 @@ class DrupalDefaultEntityController implements DrupalEntityControllerInterface { /** * Implements DrupalEntityControllerInterface::resetCache(). */ - public function resetCache() { - $this->entityCache = array(); + public function resetCache(array $ids = NULL) { + if (isset($ids)) { + foreach ($ids as $id) { + unset($this->entityCache[$id]); + } + } + else { + $this->entityCache = array(); + } } /** diff --git includes/file.inc includes/file.inc index 742c20e..e8db634 100644 --- includes/file.inc +++ includes/file.inc @@ -574,6 +574,11 @@ function file_save(stdClass $file) { $file->timestamp = REQUEST_TIME; $file->filesize = filesize($file->uri); + // Load the stored entity, if any. + if (!empty($file->fid) && !isset($file->original)) { + $file->original = entity_load_unchanged('file', $file->fid); + } + if (empty($file->fid)) { drupal_write_record('file_managed', $file); // Inform modules about the newly added file. @@ -587,6 +592,7 @@ function file_save(stdClass $file) { module_invoke_all('entity_update', $file, 'file'); } + unset($file->original); return $file; } diff --git modules/comment/comment.module modules/comment/comment.module index aa7f2ad..dbf4ca6 100644 --- modules/comment/comment.module +++ modules/comment/comment.module @@ -1442,6 +1442,11 @@ function comment_save($comment) { $comment->node_type = 'comment_node_' . $node->type; } + // Load the stored entity, if any. + if (!empty($comment->cid) && !isset($comment->original)) { + $comment->original = entity_load_unchanged('comment', $comment->cid); + } + field_attach_presave('comment', $comment); // Allow modules to alter the comment before saving. @@ -1568,6 +1573,7 @@ function comment_save($comment) { if ($comment->status == COMMENT_PUBLISHED) { module_invoke_all('comment_publish', $comment); } + unset($comment->original); } catch (Exception $e) { $transaction->rollback('comment'); diff --git modules/node/node.admin.inc modules/node/node.admin.inc index 02891de..a86ffb6 100644 --- modules/node/node.admin.inc +++ modules/node/node.admin.inc @@ -288,6 +288,8 @@ function node_mass_update($nodes, $updates) { */ function _node_mass_update_helper($nid, $updates) { $node = node_load($nid, NULL, TRUE); + // For efficiency manually save the original node before applying any changes. + $node->original = clone $node; foreach ($updates as $name => $value) { $node->$name = $value; } diff --git modules/node/node.module modules/node/node.module index 44a2274..d7cc85d 100644 --- modules/node/node.module +++ modules/node/node.module @@ -1032,6 +1032,11 @@ function node_save($node) { $transaction = db_transaction(); try { + // Load the stored entity, if any. + if (!empty($node->nid) && !isset($node->original)) { + $node->original = entity_load_unchanged('node', $node->nid); + } + field_attach_presave('node', $node); global $user; @@ -1135,6 +1140,9 @@ function node_save($node) { // Clear internal properties. unset($node->is_new); + unset($node->original); + // Clear the static loading cache. + entity_get_controller('node')->resetCache(array($node->nid)); // Ignore slave server temporarily to give time for the // saved node to be propagated to the slave. diff --git modules/node/node.test modules/node/node.test index 27abb4f..c0b6096 100644 --- modules/node/node.test +++ modules/node/node.test @@ -1041,14 +1041,14 @@ class NodeSaveTestCase extends DrupalWebTestCase { $changed = $node->changed; node_save($node); - $node = $this->drupalGetNodeByTitle($edit['title']); + $node = $this->drupalGetNodeByTitle($edit['title'], TRUE); $this->assertEqual($node->created, $created, t('Updating a node preserves "created" timestamp.')); // Programmatically set the timestamps using hook_node_presave. $node->title = 'testing_node_presave'; node_save($node); - $node = $this->drupalGetNodeByTitle('testing_node_presave'); + $node = $this->drupalGetNodeByTitle('testing_node_presave', TRUE); $this->assertEqual($node->created, 280299600, t('Saving a node uses "created" timestamp set in presave hook.')); $this->assertEqual($node->changed, 979534800, t('Saving a node uses "changed" timestamp set in presave hook.')); @@ -1071,10 +1071,40 @@ class NodeSaveTestCase extends DrupalWebTestCase { $node->changed = 280299600; node_save($node); - $node = $this->drupalGetNodeByTitle($edit['title']); + $node = $this->drupalGetNodeByTitle($edit['title'], TRUE); $this->assertEqual($node->created, 979534800, t('Updating a node uses user-set "created" timestamp.')); $this->assertNotEqual($node->changed, 280299600, t('Updating a node doesn\'t use user-set "changed" timestamp.')); } + + /** + * Tests determing changes in hook_node_presave() and verifies the static node + * load cache is cleared upon save. + */ + function testDeterminingChanges() { + // Initial creation. + $node = (object) array( + 'uid' => $this->web_user->uid, + 'type' => 'article', + 'title' => 'test_changes', + ); + node_save($node); + + // Update the node without applying changes. + node_save($node); + $this->assertEqual($node->title, 'test_changes', 'No changes have been determined.'); + + // Apply changes. + $node->title = 'updated'; + node_save($node); + + // The hook implementations node_test_node_presave() and + // node_test_node_update() determine changes and change the title. + $this->assertEqual($node->title, 'updated_presave_update', 'Changes have been determined.'); + + // Test the static node load cache to be cleared. + $node = node_load($node->nid); + $this->assertEqual($node->title, 'updated_presave', 'Static cache has been cleared.'); + } } /** diff --git modules/node/tests/node_test.module modules/node/tests/node_test.module index c32bc1e..821f48c 100644 --- modules/node/tests/node_test.module +++ modules/node/tests/node_test.module @@ -131,4 +131,22 @@ function node_test_node_presave($node) { // Drupal 1.0 release. $node->changed = 979534800; } + // Determine changes. + if (!empty($node->original) && $node->original->title == 'test_changes') { + if ($node->original->title != $node->title) { + $node->title .= '_presave'; + } + } +} + +/** + * Implements hook_node_update(). + */ +function node_test_node_update($node) { + // Determine changes on update. + if (!empty($node->original) && $node->original->title == 'test_changes') { + if ($node->original->title != $node->title) { + $node->title .= '_update'; + } + } } diff --git modules/simpletest/drupal_web_test_case.php modules/simpletest/drupal_web_test_case.php index e50adda..5146941 100644 --- modules/simpletest/drupal_web_test_case.php +++ modules/simpletest/drupal_web_test_case.php @@ -799,12 +799,14 @@ class DrupalWebTestCase extends DrupalTestCase { * * @param title * A node title, usually generated by $this->randomName(). + * @param $reset + * (optional) Whether to reset the internal node_load() cache. * * @return * A node object matching $title. */ - function drupalGetNodeByTitle($title) { - $nodes = node_load_multiple(array(), array('title' => $title)); + function drupalGetNodeByTitle($title, $reset = FALSE) { + $nodes = node_load_multiple(array(), array('title' => $title), $reset); // Load the first node returned from the database. $returned_node = reset($nodes); return $returned_node; diff --git modules/simpletest/tests/file.test modules/simpletest/tests/file.test index 36a7170..c9577c6 100644 --- modules/simpletest/tests/file.test +++ modules/simpletest/tests/file.test @@ -1485,7 +1485,7 @@ class FileMoveTest extends FileHookTestCase { $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file correctly written.')); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('move', 'update')); + $this->assertFileHooksCalled(array('move', 'load', 'update')); // Make sure we got the same file back. $this->assertEqual($source->fid, $result->fid, t("Source file id's' %fid is unchanged after move.", array('%fid' => $source->fid))); @@ -1517,7 +1517,7 @@ class FileMoveTest extends FileHookTestCase { $this->assertEqual($contents, file_get_contents($result->uri), t('Contents of file correctly written.')); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('move', 'update')); + $this->assertFileHooksCalled(array('move', 'load', 'update')); // Compare the returned value to what made it into the database. $this->assertFileUnchanged($result, file_load($result->fid, TRUE)); @@ -1891,7 +1891,7 @@ class FileSaveTest extends FileHookTestCase { $resaved_file = file_save($saved_file); // Check that the correct hooks were called. - $this->assertFileHooksCalled(array('update')); + $this->assertFileHooksCalled(array('load', 'update')); $this->assertEqual($resaved_file->fid, $saved_file->fid, t("The file ID of an existing file is not changed when updating the database."), 'File'); $this->assertTrue($resaved_file->timestamp >= $saved_file->timestamp, t("Timestamp didn't go backwards."), 'File'); diff --git modules/taxonomy/taxonomy.module modules/taxonomy/taxonomy.module index fd473e2..a78436e 100644 --- modules/taxonomy/taxonomy.module +++ modules/taxonomy/taxonomy.module @@ -388,9 +388,15 @@ function taxonomy_vocabulary_save($vocabulary) { if (!empty($vocabulary->name)) { $vocabulary->name = trim($vocabulary->name); } - // For existing vocabularies, make sure we can detect machine name changes. - if (!empty($vocabulary->vid) && !isset($vocabulary->old_machine_name)) { - $vocabulary->old_machine_name = db_query("SELECT machine_name FROM {taxonomy_vocabulary} WHERE vid = :vid", array(':vid' => $vocabulary->vid))->fetchField(); + // Load the stored entity, if any. + if (!empty($vocabulary->vid)) { + if (!isset($vocabulary->original)) { + $vocabulary->original = entity_load_unchanged('taxonomy_vocabulary', $vocabulary->vid); + } + // Make sure machine name changes are easily detected. + // @todo: Remove in Drupal 8, as it is deprecated by directly reading from + // $vocabulary->original. + $vocabulary->old_machine_name = $vocabulary->original->machine_name; } if (!isset($vocabulary->module)) { @@ -414,8 +420,9 @@ function taxonomy_vocabulary_save($vocabulary) { module_invoke_all('entity_insert', $vocabulary, 'taxonomy_vocabulary'); } + unset($vocabulary->original); cache_clear_all(); - entity_get_controller('taxonomy_vocabulary')->resetCache(); + entity_get_controller('taxonomy_vocabulary')->resetCache(array($vocabulary->vid)); return $status; } @@ -441,14 +448,14 @@ function taxonomy_vocabulary_delete($vid) { db_delete('taxonomy_vocabulary') ->condition('vid', $vid) ->execute(); - + field_attach_delete_bundle('taxonomy_term', $vocabulary->machine_name); module_invoke_all('taxonomy_vocabulary_delete', $vocabulary); module_invoke_all('entity_delete', $vocabulary, 'taxonomy_vocabulary'); - + cache_clear_all(); entity_get_controller('taxonomy_vocabulary')->resetCache(); - + return SAVED_DELETED; } catch (Exception $e) { @@ -540,6 +547,11 @@ function taxonomy_term_save($term) { $term->vocabulary_machine_name = $vocabulary->machine_name; } + // Load the stored entity, if any. + if (!empty($term->tid) && !isset($term->original)) { + $term->original = entity_load_unchanged('taxonomy_term', $term->tid); + } + field_attach_presave('taxonomy_term', $term); module_invoke_all('taxonomy_term_presave', $term); @@ -593,6 +605,7 @@ function taxonomy_term_save($term) { // Invoke the taxonomy hooks. module_invoke_all("taxonomy_term_$op", $term); module_invoke_all("entity_$op", $term, 'taxonomy_term'); + unset($term->original); return $status; } diff --git modules/user/user.module modules/user/user.module index 96c26e0..90d398d 100644 --- modules/user/user.module +++ modules/user/user.module @@ -405,6 +405,11 @@ function user_save($account, $edit = array(), $category = 'account') { unset($edit['pass']); } + // Load the stored entity, if any. + if (!empty($account->uid) && !isset($account->original)) { + $account->original = entity_load_unchanged('user', $account->uid); + } + // Presave field allowing changing of $edit. $edit = (object) $edit; field_attach_presave('user', $edit); @@ -503,6 +508,10 @@ function user_save($account, $edit = array(), $category = 'account') { // Refresh user object. $user = user_load($account->uid, TRUE); + // Make the original, unchanged user account available to update hooks. + if (isset($account->original)) { + $user->original = $account->original; + } // Send emails after we have the new user object. if (isset($edit['status']) && $edit['status'] != $account->status) { @@ -513,6 +522,7 @@ function user_save($account, $edit = array(), $category = 'account') { user_module_invoke('update', $edit, $user, $category); module_invoke_all('entity_update', $user, 'user'); + unset($user->original); } else { // Allow 'uid' to be set by the caller. There is no danger of writing an @@ -3054,6 +3064,9 @@ function user_user_operations_block($accounts) { foreach ($accounts as $account) { // Skip blocking user if they are already blocked. if ($account !== FALSE && $account->status == 1) { + // For efficiency manually save the original account before applying any + // changes. + $account->original = clone $account; user_save($account, array('status' => 0)); } } @@ -3074,6 +3087,9 @@ function user_multiple_role_edit($accounts, $operation, $rid) { // Skip adding the role to the user if they already have it. if ($account !== FALSE && !isset($account->roles[$rid])) { $roles = $account->roles + array($rid => $role_name); + // For efficiency manually save the original account before applying + // any changes. + $account->original = clone $account; user_save($account, array('roles' => $roles)); } } @@ -3084,6 +3100,9 @@ function user_multiple_role_edit($accounts, $operation, $rid) { // Skip removing the role from the user if they already don't have it. if ($account !== FALSE && isset($account->roles[$rid])) { $roles = array_diff($account->roles, array($rid => $role_name)); + // For efficiency manually save the original account before applying + // any changes. + $account->original = clone $account; user_save($account, array('roles' => $roles)); } }