Greetings,

Problem description
------------------
I have 2 node types ADVENTURE and COUNTRY. Adventure node type has a Nodereference field to a Country (means Adventure belongs to one Country). I want to overload hook_nodeapi() for an Adventure and read all information about its country and insert it into the Adventure node. Here is a piece of code I created in my module:

function mwtravel_nodeapi(&$node, $op, $teaser, $page) {
  switch ($op) {
    case 'load':
      if ('adventure' == $node->type) {
        $country = node_load($node->field_adventure_country[0]['nid']);
        content_view($country);
        $node->country = $country;
      }
    break;
  }
}

The problem is that there is no $node->field_adventure_country[0]['nid'] field at all. My module has bigger weight than content.module but looks like CCK doesn't populate node fields into $node class on hook_nodeapi(). Later I found that this problems occurs only when CCK loads cached date. After clearing cache_content table everything works fine but fails on the next load.

In the content_load($node) function I found that you fill $node with fields only when they are built for the first time but not when they are taken from cache.

if ($cached = cache_get($cid, 'cache_content')) {
  return unserialize($cached->data);
}

In this case there is no way for other modules to overload hook_nodeapi and do something really useful with CCK fields. Since CCK is a "key module" today and many modules relates on it I propose to change this to the following code:

if ($cached = cache_get($cid, 'cache_content')) {
  $extra = unserialize($cached->data); 
  foreach ($extra as $key => $value) {
    $node->$key = $value;
  }
}

I also found that you fill $node class only after _content_field_invoke_default() but not after _content_field_invoke() which is strange. I saw you had fixed it in CCK 6 version. In this case I think content_load() should not return anything at all.

Further thoughts
----------------

As I understood, hook_nodeapi('load') has 2 ways to populate data:
1. return set of fields (which are than will be inserted into $node class by node_load() function:

    if ($extra = node_invoke_nodeapi($node, 'load')) {
      foreach ($extra as $key => $value) {
        $node->$key = $value;
      }
    }

In this case all data will be available in the end of node_load() call and NOT available during hook_nodeapi(load) for other modules.

2. Populate data right into $node class. In this case data will already be inside $node class and it is probably no need to return it because node_load function will insert everything into the $node class for the second time.

According to this, modules should decide either to return on hook_nodeapi('load') or insert data into $node class.
Since there is no difference in performance and memory usage I think that populating $node class is better than returning.

Your thoughts?
Thanks.

Comments

jscheel’s picture

I absolutely agree with this. I'm not sure why the return method still exists, but it's probably just legacy code.

dopry’s picture

Status: Active » Fixed

fixed in DRUPAL-5.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.