taxonomy_node_get_terms() needs a $reset param that node_load can call when *it* is reset

Ok, I think I can expand on this and I think it is a bug. Even when the $reset parameter in node_load() is set to TRUE, old taxonomy still appears to be cached.

Having moved the part that processes the taxonomy to an entirely separate loop, I discovered now the first AND last terms are saving, rather than just the first as before. Looking at the before and after node objects of each loop, I can see the node_load is loading the wrong (initial) taxonomy every time, even though in the interim taxonomy was updated. My initial node_save saves taxonomy successfully, the following node_load brings it back successfully, the second node_save succeeds (I can prove this) but after the second node_load our taxonomy seems to still be cached from the previous one, in spite of the reset. Consequently, the next node_save overwrites the successful prior save with the NEXT term to be processed, having failed to load in and retain it.

So there must be some caching within taxonomy that ignores the node.module's request for a reset of the cache. I'll see if I can find out precisely what.

And here is the same issue, referenced elsewhere: #471074: Taxonomy synchronization caching bug

@stewsnooze Spot on! That's where the bug lies - that sneaky static.

Worked around it by binning the API (it's an import module, so code maintenance not an issue). So this:

  $existing_terms = $node->taxonomy;

Became this:

  // existing taxonomy terms must be loaded from the db
  // node_load can't get around caching problems with taxonomy.module
  $result = db_query("SELECT * FROM {term_node} WHERE nid = %d AND vid = %d", $node->nid, $node->vid);
  while ($term = db_fetch_object($result)) {
    $existing_terms[] = $term;
  }

Works now, but taxonomy_node_get_terms really needs a $reset that can be called by node_load when it's $reset == TRUE!

Updating title to suit.

Doing it like node_load() would be more consistent:

function taxonomy_node_get_terms($node, $key = 'tid', $reset = FALSE) {
  static $terms = array();

  if ($reset) {
    $terms = array();
  }

Would that to it? It would make more sense because it would follow the same pattern as this:
http://api.drupal.org/api/function/node_load/6

Something like this, no?

function taxonomy_node_get_terms($node, $key = 'tid', $reset = FALSE) {
  static $terms;

  if ($reset) {
    unset($terms[$node->vid][$key]);
  }

  if (!isset($terms[$node->vid][$key])) {
    $result = db_query(db_rewrite_sql('SELECT t.* FROM {term_node} r INNER JOIN {term_data} t ON r.tid = t.tid INNER JOIN {vocabulary} v ON t.vid = v.vid WHERE r.vid = %d ORDER BY v.weight, t.weight, t.name', 't', 'tid'), $node->vid);
    $terms[$node->vid][$key] = array();
    while ($term = db_fetch_object($result)) {
      $terms[$node->vid][$key][$term->$key] = $term;
    }
  }
  return $terms[$node->vid][$key];
}

Mind you, I can't see this ever getting committed, since all focus is on Drupal 7 and this function no longer even exists once Drupal 6 dies out.

Edit: And yes, there will then need to be a follow-up patch to tackle the hook_nodeapi() implementation.