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
Comment #1
jscheel commentedI absolutely agree with this. I'm not sure why the return method still exists, but it's probably just legacy code.
Comment #2
dopry commentedfixed in DRUPAL-5.
Comment #3
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.