- Patch #651240 by fago, sun: allow modules to react to changes to an entity.

includes/common.inc

@@ -7333,6 +7333,28 @@ function entity_load($entity_type, $ids = FALSE, $conditions = array(), $reset =
   return entity_get_controller($entity_type)->load($ids, $conditions);
 }
 
+/**
+ * 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.
  */

includes/entity.inc

@@ -24,8 +24,12 @@ public function __construct($entityType);
 
   /**
    * 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 @@ public function __construct($entityType) {
   /**
    * 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();
+    }
   }
 
   /**

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;
 }
 

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');

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;
   }

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.

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.');
+  }
 }
 
 /**

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';
+    }
+  }
 }

modules/simpletest/drupal_web_test_case.php

@@ -799,12 +799,14 @@ function __construct($test_id = NULL) {
    *
    * @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;

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');

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;
 }

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));
         }
       }