From a606ddec02ffb3f140a004656f8cd1b3a50958fb Mon Sep 17 00:00:00 2001 From: Dean Reilly Date: Sun, 17 Jun 2012 14:36:47 +0100 Subject: [PATCH] Issue #1146244: Node operation hook are called after node access records are written. Included tests. --- modules/node/node.module | 6 +- modules/node/node.test | 25 ++++++++++++ modules/node/tests/node_test.module | 72 ++++++++++++++++++++++++++++++++++- 3 files changed, 98 insertions(+), 5 deletions(-) diff --git a/modules/node/node.module b/modules/node/node.module index 71ea3b9..8273f86 100644 --- a/modules/node/node.module +++ b/modules/node/node.module @@ -1130,14 +1130,14 @@ function node_save($node) { $function = "field_attach_$op"; $function('node', $node); - module_invoke_all('node_' . $op, $node); - module_invoke_all('entity_' . $op, $node, 'node'); - // Update the node access table for this node. There's no need to delete // existing records if the node is new. $delete = $op == 'update'; node_access_acquire_grants($node, $delete); + module_invoke_all('node_' . $op, $node); + module_invoke_all('entity_' . $op, $node, 'node'); + // Clear internal properties. unset($node->is_new); unset($node->original); diff --git a/modules/node/node.test b/modules/node/node.test index 37d05e5..70411a9 100644 --- a/modules/node/node.test +++ b/modules/node/node.test @@ -922,6 +922,10 @@ class NodeAccessRecordsTestCase extends DrupalWebTestCase { // hook_node_access_records(), hook_node_grants_alter() and // hook_node_access_records_alter(). parent::setUp('node_test'); + + // Create more content types for testing. + $this->drupalCreateContentType(array('type' => 'insert_hook_order_test', 'name' => 'Insert Hook Order Test Content Type')); + $this->drupalCreateContentType(array('type' => 'update_hook_order_test', 'name' => 'Update Hook Order Test Content Type')); } /** @@ -985,6 +989,27 @@ class NodeAccessRecordsTestCase extends DrupalWebTestCase { $node6 = $this->drupalCreateNode(array('status' => 0, 'disable_node_access' => TRUE)); $records = db_query('SELECT realm, gid FROM {node_access} WHERE nid = :nid', array(':nid' => $node6->nid))->fetchAll(); $this->assertEqual(count($records), 0, t('Returned no records for unpublished node.')); + + // Create a insert hook order test node. + $node7 = $this->drupalCreateNode(array('type' => 'insert_hook_order_test')); + $this->assertTrue(node_load($node7->nid), t('Hook order test node created.')); + + // Check to see if grant was updated by node_test_node_insert + $records = db_query('SELECT realm, gid FROM {node_access} WHERE nid = :nid', array(':nid' => $node7->nid))->fetchAll(); + $this->assertEqual(count($records), 1, t('Returned the correct number of rows.')); + $this->assertEqual($records[0]->realm, 'test_hook_order_realm', t('Grant with test_hook_order_realm acquired for node.')); + $this->assertEqual($records[0]->gid, 2, t('Altered grant with gid = 2 acquired for node.')); + + // Create a update hook order test node. + $node8 = $this->drupalCreateNode(array('type' => 'update_hook_order_test')); + node_save($node8); + $this->assertTrue(node_load($node8->nid), t('Hook order test node created.')); + + // Check to see if grant was updated by node_test_node_update + $records = db_query('SELECT realm, gid FROM {node_access} WHERE nid = :nid', array(':nid' => $node8->nid))->fetchAll(); + $this->assertEqual(count($records), 1, t('Returned the correct number of rows.')); + $this->assertEqual($records[0]->realm, 'test_hook_order_realm', t('Grant with test_hook_order_realm acquired for node.')); + $this->assertEqual($records[0]->gid, 2, t('Altered grant with gid = 2 acquired for node.')); } } diff --git a/modules/node/tests/node_test.module b/modules/node/tests/node_test.module index b0ebc14..aa65dcb 100644 --- a/modules/node/tests/node_test.module +++ b/modules/node/tests/node_test.module @@ -60,6 +60,7 @@ function node_test_node_grants($account, $op) { 'test_article_realm' => array(1), 'test_page_realm' => array(1), 'test_alter_realm' => array(2), + 'test_hook_order_realm' => array(1, 2), ); } @@ -94,6 +95,44 @@ function node_test_node_access_records($node) { 'priority' => 0, ); } + elseif ($node->type == 'insert_hook_order_test') { + // We vary the gid based on some property of the node which is changed + // by hook_insert. In this example the title. + if (!preg_match('/^.* \(nid:\d+\)$/', $node->title)) { + $gid = 1; + } else { + $gid = 2; + } + + // Create grant in arbitrary test_hook_order_realm for insert hook order + // test nodes. + $grants[] = array( + 'realm' => 'test_hook_order_realm', + 'gid' => $gid, + 'grant_view' => 1, + 'grant_update' => 0, + 'grant_delete' => 0, + 'priority' => 0, + ); + } + elseif ($node->type == 'update_hook_order_test') { + if (substr($node->title, -7) != '_update') { + $gid = 1; + } else { + $gid = 2; + } + + // Create grant in arbitrary test_hook_order_realm for update hook order + // test nodes. + $grants[] = array( + 'realm' => 'test_hook_order_realm', + 'gid' => $gid, + 'grant_view' => 1, + 'grant_update' => 0, + 'grant_delete' => 0, + 'priority' => 0, + ); + } return $grants; } @@ -103,7 +142,8 @@ function node_test_node_access_records($node) { function node_test_node_access_records_alter(&$grants, $node) { if (!empty($grants)) { foreach ($grants as $key => $grant) { - // Alter grant from test_page_realm to test_alter_realm and modify the gid. + // Alter grant from test_page_realm to test_alter_realm and modify the + // gid. if ($grant['realm'] == 'test_page_realm' && $node->promote) { $grants[$key]['realm'] = 'test_alter_realm'; $grants[$key]['gid'] = 2; @@ -139,11 +179,39 @@ function node_test_node_presave($node) { } /** + * Implements hook_node_insert(). + */ +function node_test_node_insert($node) { + if ($node->type == 'insert_hook_order_test') { + // We clone the original node object and save it rather than updating the + // original. This ensures that the node_access_records function receives + // different node objects independent of the order of hooks in + // $node->save() and allows us to test that the correct data is stored. + // + // This is unneccessary in this use case but may not be in more complex + // siutations. + $updated_node = clone $node; + + // Including the nid is an often requested task so we'll use that to + // demonstrate. + $updated_node->title .= ' (nid:' . $updated_node->nid . ')'; + unset($updated_node->is_new); + node_save($updated_node); + } +} + +/** * Implements hook_node_update(). */ function node_test_node_update($node) { + if ($node->type == 'update_hook_order_test' && substr($node->title, -7) != '_update') { + // See the node_test_node_insert comments for why we clone the node object. + $updated_node = clone $node; + $updated_node->title .= '_update'; + node_save($updated_node); + } // Determine changes on update. - if (!empty($node->original) && $node->original->title == 'test_changes') { + elseif (!empty($node->original) && $node->original->title == 'test_changes') { if ($node->original->title != $node->title) { $node->title .= '_update'; } -- 1.7.5.4