Commit a3ebad06 authored by catch's avatar catch
Browse files

Issue #1346032 by xjm, aspilicious: Fixed Ordering of hook_entity_delete() is inconsistent.

parent 463a710b
...@@ -1225,6 +1225,7 @@ function file_create_filename($basename, $directory) { ...@@ -1225,6 +1225,7 @@ function file_create_filename($basename, $directory) {
* @see file_unmanaged_delete() * @see file_unmanaged_delete()
* @see file_usage_list() * @see file_usage_list()
* @see file_usage_delete() * @see file_usage_delete()
* @see hook_file_predelete()
* @see hook_file_delete() * @see hook_file_delete()
*/ */
function file_delete(stdClass $file, $force = FALSE) { function file_delete(stdClass $file, $force = FALSE) {
...@@ -1246,15 +1247,20 @@ function file_delete(stdClass $file, $force = FALSE) { ...@@ -1246,15 +1247,20 @@ function file_delete(stdClass $file, $force = FALSE) {
return $references; return $references;
} }
// Let other modules clean up any references to the deleted file. // Let other modules clean up any references to the file prior to deletion.
module_invoke_all('file_delete', $file); module_invoke_all('file_predelete', $file);
module_invoke_all('entity_delete', $file, 'file'); module_invoke_all('entity_predelete', $file, 'file');
// Make sure the file is deleted before removing its row from the // Make sure the file is deleted before removing its row from the
// database, so UIs can still find the file in the database. // database, so UIs can still find the file in the database.
if (file_unmanaged_delete($file->uri)) { if (file_unmanaged_delete($file->uri)) {
db_delete('file_managed')->condition('fid', $file->fid)->execute(); db_delete('file_managed')->condition('fid', $file->fid)->execute();
db_delete('file_usage')->condition('fid', $file->fid)->execute(); db_delete('file_usage')->condition('fid', $file->fid)->execute();
// Let other modules respond to file deletion.
module_invoke_all('file_delete', $file);
module_invoke_all('entity_delete', $file, 'file');
return TRUE; return TRUE;
} }
return FALSE; return FALSE;
......
...@@ -959,9 +959,9 @@ function book_node_update($node) { ...@@ -959,9 +959,9 @@ function book_node_update($node) {
} }
/** /**
* Implements hook_node_delete(). * Implements hook_node_predelete().
*/ */
function book_node_delete($node) { function book_node_predelete($node) {
if (!empty($node->book['bid'])) { if (!empty($node->book['bid'])) {
if ($node->nid == $node->book['bid']) { if ($node->nid == $node->book['bid']) {
// Handle deletion of a top-level post. // Handle deletion of a top-level post.
......
...@@ -129,12 +129,39 @@ function hook_comment_unpublish($comment) { ...@@ -129,12 +129,39 @@ function hook_comment_unpublish($comment) {
} }
/** /**
* The comment is being deleted by the moderator. * Act before comment deletion.
*
* This hook is invoked from comment_delete_multiple() before
* field_attach_delete() is called and before the comment is actually removed
* from the database.
* *
* @param $comment * @param $comment
* Passes in the comment the action is being performed on. * The comment object for the comment that is about to be deleted.
* @return *
* Nothing. * @see hook_comment_delete()
* @see comment_delete_multiple()
* @see entity_delete_multiple()
*/
function hook_comment_predelete($comment) {
// Delete a record associated with the comment in a custom table.
db_delete('example_comment_table')
->condition('cid', $comment->cid)
->execute();
}
/**
* Respond to comment deletion.
*
* This hook is invoked from comment_delete_multiple() after
* field_attach_delete() has called and after the comment has been removed from
* the database.
*
* @param $comment
* The comment object for the comment that has been deleted.
*
* @see hook_comment_predelete()
* @see comment_delete_multiple()
* @see entity_delete_multiple()
*/ */
function hook_comment_delete($comment) { function hook_comment_delete($comment) {
drupal_set_message(t('Comment: @subject has been deleted', array('@subject' => $comment->subject))); drupal_set_message(t('Comment: @subject has been deleted', array('@subject' => $comment->subject)));
......
...@@ -1304,9 +1304,9 @@ function comment_node_insert($node) { ...@@ -1304,9 +1304,9 @@ function comment_node_insert($node) {
} }
/** /**
* Implements hook_node_delete(). * Implements hook_node_predelete().
*/ */
function comment_node_delete($node) { function comment_node_predelete($node) {
$cids = db_query('SELECT cid FROM {comment} WHERE nid = :nid', array(':nid' => $node->nid))->fetchCol(); $cids = db_query('SELECT cid FROM {comment} WHERE nid = :nid', array(':nid' => $node->nid))->fetchCol();
comment_delete_multiple($cids); comment_delete_multiple($cids);
db_delete('node_comment_statistics') db_delete('node_comment_statistics')
...@@ -1402,9 +1402,9 @@ function comment_user_cancel($edit, $account, $method) { ...@@ -1402,9 +1402,9 @@ function comment_user_cancel($edit, $account, $method) {
} }
/** /**
* Implements hook_user_delete(). * Implements hook_user_predelete().
*/ */
function comment_user_delete($account) { function comment_user_predelete($account) {
$cids = db_query('SELECT c.cid FROM {comment} c WHERE uid = :uid', array(':uid' => $account->uid))->fetchCol(); $cids = db_query('SELECT c.cid FROM {comment} c WHERE uid = :uid', array(':uid' => $account->uid))->fetchCol();
comment_delete_multiple($cids); comment_delete_multiple($cids);
} }
...@@ -1457,6 +1457,9 @@ function comment_delete($cid) { ...@@ -1457,6 +1457,9 @@ function comment_delete($cid) {
* *
* @param $cids * @param $cids
* The comment to delete. * The comment to delete.
*
* @see hook_comment_predelete()
* @see hook_comment_delete()
*/ */
function comment_delete_multiple($cids) { function comment_delete_multiple($cids) {
entity_delete_multiple('comment', $cids); entity_delete_multiple('comment', $cids);
......
...@@ -270,7 +270,7 @@ function hook_entity_insert($entity, $type) { ...@@ -270,7 +270,7 @@ function hook_entity_insert($entity, $type) {
* @param $entity * @param $entity
* The entity object. * The entity object.
* @param $type * @param $type
* The type of entity being updated (i.e. node, user, comment). * The type of entity being updated (e.g. node, user, comment).
*/ */
function hook_entity_update($entity, $type) { function hook_entity_update($entity, $type) {
// Update the entity's entry in a fictional table of all entities. // Update the entity's entry in a fictional table of all entities.
...@@ -286,10 +286,42 @@ function hook_entity_update($entity, $type) { ...@@ -286,10 +286,42 @@ function hook_entity_update($entity, $type) {
} }
/** /**
* Act on entities when deleted. * Act before entity deletion.
*
* This hook runs after the entity type-specific predelete hook.
* *
* @param $entity * @param $entity
* The entity object. * The entity object for the entity that is about to be deleted.
* @param $type
* The type of entity being deleted (e.g. node, user, comment).
*/
function hook_entity_predelete($entity, $type) {
// Count references to this entity in a custom table before they are removed
// upon entity deletion.
list($id) = entity_extract_ids($type, $entity);
$count = db_select('example_entity_data')
->condition('type', $type)
->condition('id', $id)
->countQuery()
->execute()
->fetchField();
// Log the count in a table that records this statistic for deleted entities.
$ref_count_record = (object) array(
'count' => $count,
'type' => $type,
'id' => $id,
);
drupal_write_record('example_deleted_entity_statistics', $ref_count_record);
}
/**
* Respond to entity deletion.
*
* This hook runs after the entity type-specific delete hook.
*
* @param $entity
* The entity object for the entity that has been deleted.
* @param $type * @param $type
* The type of entity being deleted (i.e. node, user, comment). * The type of entity being deleted (i.e. node, user, comment).
*/ */
......
...@@ -454,6 +454,9 @@ public function delete($ids) { ...@@ -454,6 +454,9 @@ public function delete($ids) {
try { try {
$this->preDelete($entities); $this->preDelete($entities);
foreach ($entities as $id => $entity) {
$this->invokeHook('predelete', $entity);
}
$ids = array_keys($entities); $ids = array_keys($entities);
db_delete($this->entityInfo['base table']) db_delete($this->entityInfo['base table'])
...@@ -548,8 +551,8 @@ protected function postDelete($entities) { } ...@@ -548,8 +551,8 @@ protected function postDelete($entities) { }
/** /**
* Invokes a hook on behalf of the entity. * Invokes a hook on behalf of the entity.
* *
* @param $op * @param $hook
* One of 'presave', 'insert', 'update', or 'delete'. * One of 'presave', 'insert', 'update', 'predelete', or 'delete'.
* @param $entity * @param $entity
* The entity object. * The entity object.
*/ */
......
...@@ -212,6 +212,59 @@ function entity_crud_hook_test_user_update() { ...@@ -212,6 +212,59 @@ function entity_crud_hook_test_user_update() {
$_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called'); $_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called');
} }
//
// Predelete hooks
//
/**
* Implements hook_entity_predelete().
*/
function entity_crud_hook_test_entity_predelete($entity, $type) {
$_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called for type ' . $type);
}
/**
* Implements hook_comment_predelete().
*/
function entity_crud_hook_test_comment_predelete() {
$_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called');
}
/**
* Implements hook_file_predelete().
*/
function entity_crud_hook_test_file_predelete() {
$_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called');
}
/**
* Implements hook_node_predelete().
*/
function entity_crud_hook_test_node_predelete() {
$_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called');
}
/**
* Implements hook_taxonomy_term_predelete().
*/
function entity_crud_hook_test_taxonomy_term_predelete() {
$_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called');
}
/**
* Implements hook_taxonomy_vocabulary_predelete().
*/
function entity_crud_hook_test_taxonomy_vocabulary_predelete() {
$_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called');
}
/**
* Implements hook_user_predelete().
*/
function entity_crud_hook_test_user_predelete() {
$_SESSION['entity_crud_hook_test'][] = (__FUNCTION__ . ' called');
}
// //
// Delete hooks // Delete hooks
// //
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
* - hook_entity_insert() * - hook_entity_insert()
* - hook_entity_load() * - hook_entity_load()
* - hook_entity_update() * - hook_entity_update()
* - hook_entity_predelete()
* - hook_entity_delete() * - hook_entity_delete()
* As well as all type-specific hooks, like hook_node_insert(), * As well as all type-specific hooks, like hook_node_insert(),
* hook_comment_update(), etc. * hook_comment_update(), etc.
...@@ -27,24 +28,28 @@ class EntityCrudHookTestCase extends DrupalWebTestCase { ...@@ -27,24 +28,28 @@ class EntityCrudHookTestCase extends DrupalWebTestCase {
} }
/** /**
* Pass if the message $text was set by one of the CRUD hooks in * Checks the order of CRUD hook execution messages.
* entity_crud_hook_test.module, i.e., if the $text is an element of
* $_SESSION['entity_crud_hook_test'].
* *
* @param $text * entity_crud_hook_test.module implements all core entity CRUD hooks and
* Plain text to look for. * stores a message for each in $_SESSION['entity_crud_hook_test'].
* @param $message *
* Message to display. * @param $messages
* @param $group * An array of plain-text messages in the order they should appear.
* The group this message belongs to, defaults to 'Other'.
* @return
* TRUE on pass, FALSE on fail.
*/ */
protected function assertHookMessage($text, $message = NULL, $group = 'Other') { protected function assertHookMessageOrder($messages) {
if (!isset($message)) { $positions = array();
$message = $text; foreach ($messages as $message) {
// Verify that each message is found and record its position.
$position = array_search($message, $_SESSION['entity_crud_hook_test']);
if ($this->assertTrue($position !== FALSE, $message)) {
$positions[] = $position;
}
} }
return $this->assertTrue(array_search($text, $_SESSION['entity_crud_hook_test']) !== FALSE, $message, $group);
// Sort the positions and ensure they remain in the same order.
$sorted = $positions;
sort($sorted);
$this->assertTrue($sorted == $positions, 'The hook messages appear in the correct order.');
} }
/** /**
...@@ -77,34 +82,45 @@ class EntityCrudHookTestCase extends DrupalWebTestCase { ...@@ -77,34 +82,45 @@ class EntityCrudHookTestCase extends DrupalWebTestCase {
'status' => 1, 'status' => 1,
'language' => LANGUAGE_NONE, 'language' => LANGUAGE_NONE,
)); ));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
comment_save($comment); comment_save($comment);
$this->assertHookMessage('entity_crud_hook_test_entity_presave called for type comment'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_comment_presave called'); 'entity_crud_hook_test_comment_presave called',
$this->assertHookMessage('entity_crud_hook_test_entity_insert called for type comment'); 'entity_crud_hook_test_entity_presave called for type comment',
$this->assertHookMessage('entity_crud_hook_test_comment_insert called'); 'entity_crud_hook_test_comment_insert called',
'entity_crud_hook_test_entity_insert called for type comment',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
$comment = comment_load($comment->cid); $comment = comment_load($comment->cid);
$this->assertHookMessage('entity_crud_hook_test_entity_load called for type comment'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_comment_load called'); 'entity_crud_hook_test_entity_load called for type comment',
'entity_crud_hook_test_comment_load called',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
$comment->subject = 'New subject'; $comment->subject = 'New subject';
comment_save($comment); comment_save($comment);
$this->assertHookMessage('entity_crud_hook_test_entity_presave called for type comment'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_comment_presave called'); 'entity_crud_hook_test_comment_presave called',
$this->assertHookMessage('entity_crud_hook_test_entity_update called for type comment'); 'entity_crud_hook_test_entity_presave called for type comment',
$this->assertHookMessage('entity_crud_hook_test_comment_update called'); 'entity_crud_hook_test_comment_update called',
'entity_crud_hook_test_entity_update called for type comment',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
comment_delete($comment->cid); comment_delete($comment->cid);
$this->assertHookMessage('entity_crud_hook_test_entity_delete called for type comment'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_comment_delete called'); 'entity_crud_hook_test_comment_predelete called',
'entity_crud_hook_test_entity_predelete called for type comment',
'entity_crud_hook_test_comment_delete called',
'entity_crud_hook_test_entity_delete called for type comment',
));
} }
/** /**
...@@ -126,31 +142,41 @@ class EntityCrudHookTestCase extends DrupalWebTestCase { ...@@ -126,31 +142,41 @@ class EntityCrudHookTestCase extends DrupalWebTestCase {
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
file_save($file); file_save($file);
$this->assertHookMessage('entity_crud_hook_test_entity_presave called for type file'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_file_presave called'); 'entity_crud_hook_test_file_presave called',
$this->assertHookMessage('entity_crud_hook_test_entity_insert called for type file'); 'entity_crud_hook_test_entity_presave called for type file',
$this->assertHookMessage('entity_crud_hook_test_file_insert called'); 'entity_crud_hook_test_file_insert called',
'entity_crud_hook_test_entity_insert called for type file',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
$file = file_load($file->fid); $file = file_load($file->fid);
$this->assertHookMessage('entity_crud_hook_test_entity_load called for type file'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_file_load called'); 'entity_crud_hook_test_entity_load called for type file',
'entity_crud_hook_test_file_load called',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
$file->filename = 'new.entity_crud_hook_test.file'; $file->filename = 'new.entity_crud_hook_test.file';
file_save($file); file_save($file);
$this->assertHookMessage('entity_crud_hook_test_entity_presave called for type file'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_file_presave called'); 'entity_crud_hook_test_file_presave called',
$this->assertHookMessage('entity_crud_hook_test_entity_update called for type file'); 'entity_crud_hook_test_entity_presave called for type file',
$this->assertHookMessage('entity_crud_hook_test_file_update called'); 'entity_crud_hook_test_file_update called',
'entity_crud_hook_test_entity_update called for type file',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
file_delete($file); file_delete($file);
$this->assertHookMessage('entity_crud_hook_test_entity_delete called for type file'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_file_delete called'); 'entity_crud_hook_test_file_predelete called',
'entity_crud_hook_test_entity_predelete called for type file',
'entity_crud_hook_test_file_delete called',
'entity_crud_hook_test_entity_delete called for type file',
));
} }
/** /**
...@@ -172,31 +198,41 @@ class EntityCrudHookTestCase extends DrupalWebTestCase { ...@@ -172,31 +198,41 @@ class EntityCrudHookTestCase extends DrupalWebTestCase {
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
node_save($node); node_save($node);
$this->assertHookMessage('entity_crud_hook_test_entity_presave called for type node'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_node_presave called'); 'entity_crud_hook_test_node_presave called',
$this->assertHookMessage('entity_crud_hook_test_entity_insert called for type node'); 'entity_crud_hook_test_entity_presave called for type node',
$this->assertHookMessage('entity_crud_hook_test_node_insert called'); 'entity_crud_hook_test_node_insert called',
'entity_crud_hook_test_entity_insert called for type node',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
$node = node_load($node->nid); $node = node_load($node->nid);
$this->assertHookMessage('entity_crud_hook_test_entity_load called for type node'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_node_load called'); 'entity_crud_hook_test_entity_load called for type node',
'entity_crud_hook_test_node_load called',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
$node->title = 'New title'; $node->title = 'New title';
node_save($node); node_save($node);
$this->assertHookMessage('entity_crud_hook_test_entity_presave called for type node'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_node_presave called'); 'entity_crud_hook_test_node_presave called',
$this->assertHookMessage('entity_crud_hook_test_entity_update called for type node'); 'entity_crud_hook_test_entity_presave called for type node',
$this->assertHookMessage('entity_crud_hook_test_node_update called'); 'entity_crud_hook_test_node_update called',
'entity_crud_hook_test_entity_update called for type node',
));
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
node_delete($node->nid); node_delete($node->nid);
$this->assertHookMessage('entity_crud_hook_test_entity_delete called for type node'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_node_delete called'); 'entity_crud_hook_test_node_predelete called',
'entity_crud_hook_test_entity_predelete called for type node',
'entity_crud_hook_test_node_delete called',
'entity_crud_hook_test_entity_delete called for type node',
));
} }
/** /**
...@@ -220,31 +256,41 @@ class EntityCrudHookTestCase extends DrupalWebTestCase { ...@@ -220,31 +256,41 @@ class EntityCrudHookTestCase extends DrupalWebTestCase {
$_SESSION['entity_crud_hook_test'] = array(); $_SESSION['entity_crud_hook_test'] = array();
taxonomy_term_save($term); taxonomy_term_save($term);
$this->assertHookMessage('entity_crud_hook_test_entity_presave called for type taxonomy_term'); $this->assertHookMessageOrder(array(
$this->assertHookMessage('entity_crud_hook_test_taxonomy_term_presave called'); 'entity_crud_hook_test_taxonomy_term_presave called',
$this->assertHookMessage('entity_crud_hook_test_entity_insert called for type taxonomy_term'); 'entity_crud_hook_test_entity_presave called for type taxonomy_term',
$this->assertHookMessage('entity_crud_hook_test_taxonomy_term_insert called'); 'entity_crud_hook_test_taxonomy_term_insert called',
'entity_crud_hook_test_entity_insert called for type taxonomy_term',