Issue #1050466 by xjm, makara, rhayun: Fixed The taxonomy index should be maintained in a node hook, not a field hook.

2 files changed, 283 insertions, 35 deletions

diff --git a/modules/taxonomy/taxonomy.module b/modules/taxonomy/taxonomy.module
index 379de71c7..891875b62 100644
--- a/modules/taxonomy/taxonomy.module
+++ b/modules/taxonomy/taxonomy.module
@@ -1724,48 +1724,75 @@ function taxonomy_field_presave($entity_type, $entity, $field, $instance, $langc
 }
 
 /**
- * Implements hook_field_insert().
+ * Implements hook_node_insert().
  */
-function taxonomy_field_insert($entity_type, $entity, $field, $instance, $langcode, &$items) {
-  // We maintain a denormalized table of term/node relationships, containing
-  // only data for current, published nodes.
-  if (variable_get('taxonomy_maintain_index_table', TRUE) && $field['storage']['type'] == 'field_sql_storage' && $entity_type == 'node' && $entity->status) {
-    $query = db_insert('taxonomy_index')->fields(array('nid', 'tid', 'sticky', 'created', ));
-    foreach ($items as $item) {
-      $query->values(array(
-        'nid' => $entity->nid,
-        'tid' => $item['tid'],
-        'sticky' => $entity->sticky,
-        'created' => $entity->created,
-      ));
-    }
-    $query->execute();
-  }
+function taxonomy_node_insert($node) {
+  // Add taxonomy index entries for the node.
+  taxonomy_build_node_index($node);
 }
 
 /**
- * Implements hook_field_update().
+ * Builds and inserts taxonomy index entries for a given node.
+ *
+ * The index lists all terms that are related to a given node entity, and is
+ * therefore maintained at the entity level.
+ *
+ * @param $node
+ *   The node object.
  */
-function taxonomy_field_update($entity_type, $entity, $field, $instance, $langcode, &$items) {
-  if (variable_get('taxonomy_maintain_index_table', TRUE) && $field['storage']['type'] == 'field_sql_storage' && $entity_type == 'node') {
-    $first_call = &drupal_static(__FUNCTION__, array());
-
-    // We don't maintain data for old revisions, so clear all previous values
-    // from the table. Since this hook runs once per field, per object, make
-    // sure we only wipe values once.
-    if (!isset($first_call[$entity->nid])) {
-      $first_call[$entity->nid] = FALSE;
-      db_delete('taxonomy_index')->condition('nid', $entity->nid)->execute();
+function taxonomy_build_node_index($node) {
+  // We maintain a denormalized table of term/node relationships, containing
+  // only data for current, published nodes.
+  $status = NULL;
+  if (variable_get('taxonomy_maintain_index_table', TRUE)) {
+    // If a node property is not set in the node object when node_save() is
+    // called, the old value from $node->original is used.
+    if (!empty($node->original)) {
+      $status = (int)(!empty($node->status) || (!isset($node->status) && !empty($node->original->status)));
+      $sticky = (int)(!empty($node->sticky) || (!isset($node->sticky) && !empty($node->original->sticky)));
+    }
+    else {
+      $status = (int)(!empty($node->status));
+      $sticky = (int)(!empty($node->sticky));
     }
-    // Only save data to the table if the node is published.
-    if ($entity->status) {
+  }
+  // We only maintain the taxonomy index for published nodes.
+  if ($status) {
+    // Collect a unique list of all the term IDs from all node fields.
+    $tid_all = array();
+    foreach (field_info_instances('node', $node->type) as $instance) {
+      $field_name = $instance['field_name'];
+      $field = field_info_field($field_name);
+      if ($field['module'] == 'taxonomy' && $field['storage']['type'] == 'field_sql_storage') {
+        // If a field value is not set in the node object when node_save() is
+        // called, the old value from $node->original is used.
+        if (isset($node->{$field_name})) {
+          $items = $node->{$field_name};
+        }
+        elseif (isset($node->original->{$field_name})) {
+          $items = $node->original->{$field_name};
+        }
+        else {
+          continue;
+        }
+        foreach (field_available_languages('node', $field) as $langcode) {
+          if (!empty($items[$langcode])) {
+            foreach ($items[$langcode] as $item) {
+              $tid_all[$item['tid']] = $item['tid'];
+            }
+          }
+        }
+      }
+    }
+    // Insert index entries for all the node's terms.
+    if (!empty($tid_all)) {
       $query = db_insert('taxonomy_index')->fields(array('nid', 'tid', 'sticky', 'created'));
-      foreach ($items as $item) {
+      foreach ($tid_all as $tid) {
         $query->values(array(
-          'nid' => $entity->nid,
-          'tid' => $item['tid'],
-          'sticky' => $entity->sticky,
-          'created' => $entity->created,
+          'nid' => $node->nid,
+          'tid' => $tid,
+          'sticky' => $sticky,
+          'created' => $node->created,
         ));
       }
       $query->execute();
@@ -1774,11 +1801,30 @@ function taxonomy_field_update($entity_type, $entity, $field, $instance, $langco
 }
 
 /**
+ * Implements hook_node_update().
+ */
+function taxonomy_node_update($node) {
+  // Always rebuild the node's taxonomy index entries on node save.
+  taxonomy_delete_node_index($node);
+  taxonomy_build_node_index($node);
+}
+
+/**
  * Implements hook_node_delete().
  */
 function taxonomy_node_delete($node) {
+  // Clean up the {taxonomy_index} table when nodes are deleted.
+  taxonomy_delete_node_index($node);
+}
+
+/**
+ * Deletes taxonomy index entries for a given node.
+ *
+ * @param $node
+ *   The node object.
+ */
+function taxonomy_delete_node_index($node) {
   if (variable_get('taxonomy_maintain_index_table', TRUE)) {
-    // Clean up the {taxonomy_index} table when nodes are deleted.
     db_delete('taxonomy_index')->condition('nid', $node->nid)->execute();
   }
 }
diff --git a/modules/taxonomy/taxonomy.test b/modules/taxonomy/taxonomy.test
index 183e808ed..03fdcb095 100644
--- a/modules/taxonomy/taxonomy.test
+++ b/modules/taxonomy/taxonomy.test
@@ -833,6 +833,208 @@ class TaxonomyTermTestCase extends TaxonomyWebTestCase {
 }
 
 /**
+ * Tests the hook implementations that maintain the taxonomy index.
+ */
+class TaxonomyTermIndexTestCase extends TaxonomyWebTestCase {
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Taxonomy term index',
+      'description' => 'Tests the hook implementations that maintain the taxonomy index.',
+      'group' => 'Taxonomy',
+    );
+  }
+
+  function setUp() {
+    parent::setUp('taxonomy');
+
+    // Create an administrative user.
+    $this->admin_user = $this->drupalCreateUser(array('administer taxonomy', 'bypass node access'));
+    $this->drupalLogin($this->admin_user);
+
+    // Create a vocabulary and add two term reference fields to article nodes.
+    $this->vocabulary = $this->createVocabulary();
+
+    $this->field_name_1 = drupal_strtolower($this->randomName());
+    $this->field_1 = array(
+      'field_name' => $this->field_name_1,
+      'type' => 'taxonomy_term_reference',
+      'cardinality' => FIELD_CARDINALITY_UNLIMITED,
+      'settings' => array(
+        'allowed_values' => array(
+          array(
+            'vocabulary' => $this->vocabulary->machine_name,
+            'parent' => 0,
+          ),
+        ),
+      ),
+    );
+    field_create_field($this->field_1);
+    $this->instance_1 = array(
+      'field_name' => $this->field_name_1,
+      'bundle' => 'article',
+      'entity_type' => 'node',
+      'widget' => array(
+        'type' => 'options_select',
+      ),
+      'display' => array(
+        'default' => array(
+          'type' => 'taxonomy_term_reference_link',
+        ),
+      ),
+    );
+    field_create_instance($this->instance_1);
+
+    $this->field_name_2 = drupal_strtolower($this->randomName());
+    $this->field_2 = array(
+      'field_name' => $this->field_name_2,
+      'type' => 'taxonomy_term_reference',
+      'cardinality' => FIELD_CARDINALITY_UNLIMITED,
+      'settings' => array(
+        'allowed_values' => array(
+          array(
+            'vocabulary' => $this->vocabulary->machine_name,
+            'parent' => 0,
+          ),
+        ),
+      ),
+    );
+    field_create_field($this->field_2);
+    $this->instance_2 = array(
+      'field_name' => $this->field_name_2,
+      'bundle' => 'article',
+      'entity_type' => 'node',
+      'widget' => array(
+        'type' => 'options_select',
+      ),
+      'display' => array(
+        'default' => array(
+          'type' => 'taxonomy_term_reference_link',
+        ),
+      ),
+    );
+    field_create_instance($this->instance_2);
+  }
+
+  /**
+   * Tests that the taxonomy index is maintained properly.
+   */
+  function testTaxonomyIndex() {
+    // Create terms in the vocabulary.
+    $term_1 = $this->createTerm($this->vocabulary);
+    $term_2 = $this->createTerm($this->vocabulary);
+
+    // Post an article.
+    $edit = array();
+    $langcode = LANGUAGE_NONE;
+    $edit["title"] = $this->randomName();
+    $edit["body[$langcode][0][value]"] = $this->randomName();
+    $edit["{$this->field_name_1}[$langcode][]"] = $term_1->tid;
+    $edit["{$this->field_name_2}[$langcode][]"] = $term_1->tid;
+    $this->drupalPost('node/add/article', $edit, t('Save'));
+
+    // Check that the term is indexed, and only once.
+    $node = $this->drupalGetNodeByTitle($edit["title"]);
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_1->tid,
+    ))->fetchField();
+    $this->assertEqual(1, $index_count, t('Term 1 is indexed once.'));
+
+    // Update the article to change one term.
+    $edit["{$this->field_name_1}[$langcode][]"] = $term_2->tid;
+    $this->drupalPost('node/' . $node->nid . '/edit', $edit, t('Save'));
+
+    // Check that both terms are indexed.
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_1->tid,
+    ))->fetchField();
+    $this->assertEqual(1, $index_count, t('Term 1 is indexed.'));
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_2->tid,
+    ))->fetchField();
+    $this->assertEqual(1, $index_count, t('Term 2 is indexed.'));
+
+    // Update the article to change another term.
+    $edit["{$this->field_name_2}[$langcode][]"] = $term_2->tid;
+    $this->drupalPost('node/' . $node->nid . '/edit', $edit, t('Save'));
+
+    // Check that only one term is indexed.
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_1->tid,
+    ))->fetchField();
+    $this->assertEqual(0, $index_count, t('Term 1 is not indexed.'));
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_2->tid,
+    ))->fetchField();
+    $this->assertEqual(1, $index_count, t('Term 2 is indexed once.'));
+
+    // Redo the above tests without interface.
+    $update_node = array(
+      'nid' => $node->nid,
+      'vid' => $node->vid,
+      'uid' => $node->uid,
+      'type' => $node->type,
+      'title' => $this->randomName(),
+    );
+
+    // Update the article with no term changed.
+    $updated_node = (object) $update_node;
+    node_save($updated_node);
+
+    // Check that the index was not changed.
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_1->tid,
+    ))->fetchField();
+    $this->assertEqual(0, $index_count, t('Term 1 is not indexed.'));
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_2->tid,
+    ))->fetchField();
+    $this->assertEqual(1, $index_count, t('Term 2 is indexed once.'));
+
+    // Update the article to change one term.
+    $update_node[$this->field_name_1][$langcode] = array(array('tid' => $term_1->tid));
+    $updated_node = (object) $update_node;
+    node_save($updated_node);
+
+    // Check that both terms are indexed.
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_1->tid,
+    ))->fetchField();
+    $this->assertEqual(1, $index_count, t('Term 1 is indexed.'));
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_2->tid,
+    ))->fetchField();
+    $this->assertEqual(1, $index_count, t('Term 2 is indexed.'));
+
+    // Update the article to change another term.
+    $update_node[$this->field_name_2][$langcode] = array(array('tid' => $term_1->tid));
+    $updated_node = (object) $update_node;
+    node_save($updated_node);
+
+    // Check that only one term is indexed.
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_1->tid,
+    ))->fetchField();
+    $this->assertEqual(1, $index_count, t('Term 1 is indexed once.'));
+    $index_count = db_query('SELECT COUNT(*) FROM {taxonomy_index} WHERE nid = :nid AND tid = :tid', array(
+      ':nid' => $node->nid,
+      ':tid' => $term_2->tid,
+    ))->fetchField();
+    $this->assertEqual(0, $index_count, t('Term 2 is not indexed.'));
+  }
+}
+
+/**
  * Test the taxonomy_term_load_multiple() function.
  */
 class TaxonomyLoadMultipleUnitTest extends TaxonomyWebTestCase {