Jump to:
| Project: | Blog reactions |
| Version: | 5.x-1.6 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | sanduhrs |
| Status: | closed (fixed) |
Issue Summary
Hi there,
it's me again :) I've activated normal caching (not aggressive) and on the frontpage I'm getting the above mentioned error " Call to undefined function arg() on line 119". In my case the lines 118 and 119 look as follows:
function blog_reactions_exit($destination = NULL) {
if (arg(0) == 'node' AND is_numeric(arg(1))) {I've checked the Drupal API and the guys are saying:
Only use this hook if your code must run even for cached page views. If you have code which must run once on all non cached pages, use hook_menu(!$may_cache) instead. Thats the usual case. If you implement this hook and see an error like 'Call to undefined function', it is likely that you are depending on the presence of a module which has not been loaded yet. It is not loaded because Drupal is still in bootstrap mode. The usual fix is to move your code to hook_menu(!$may_cache).
So what can we do? Can we move the functions to hook_menu? What do you think?
Thanks, Bjoern
Comments
#1
I cant' reproduce this.
Whether I set the caching mode to Disabled, Normal or Aggressive, doesn't make any difference.
Any more hints?
Moving the code to hook_menu() is not an option. I chose hook_exit(), because it is executed, after the page has been rendered and delivered to the browser. So the user doesn't have to wait, until we fetched the results from 4 different services and saved them to database.
The only way around hook_exit() is, implementing hook_cron().
#2
Strange that you can't reproduce the problem. I had the same problem with a module written by me. I've implemented hook_init() and the message appeared as well. I thought only using aggressive caching can cause problems. So right now I don't know what to do. Do you think it's a big problem using hook_cron()? I'll try it today. Thanks.
Edit: I think I know why my module was throwing errors. Some guys discuss it here: http://drupal.org/node/140573. If bootstrap is set in the system table it should work. I've implemented hook_init() after enabling the module. So bootstrap is still not set. I've checked my blog_reactions installation but there bootstrap is set... So I'll try moving my the functions form hook_exit() to hook_cron().
#3
So, according to [1] visiting the modules page ?q=admin/build/modules should update the system table and solve your problem.
On my system table the bootstrap field for blog_reactions ist set to 1.
Nontheless setting it to 0 manually doesn't throw any error. It still works as expected and fetches the reactions from all services.
Visiting the modules page automatically resets the value to 1.
[1] http://drupal.org/node/140573#comment-228129
#4
Okay I've checked my bootstrap settings. As I said bootstrap is set to 1 for blog_reactions. I still get the error message when I refresh the browser. Very strange. Do you know another setting which should cause the problem using normal caching? Right now I think using the code to hook_cron is a lot of work isn't it? Thanks, Bjoern
#5
Moving to hook_cron() would include:
* Set a limit on how many nodes to update per cron run
Perhaps in the settings?
* Check which nodes haven't been updated the longest
Track last nid via variable_set() and respect chosen node types
* Get next nids to update
* Craft the URI
* Run $reactions = blog_reactions_fetch($nid, $uri);
* Run blog_reactions_set_items($reactions);
#6
We have some of the things right? But it sounds like you don't want to do it because the module is working for you and there's no point for changing it?
#7
I'm short of time atm.
#8
Okay I'll see what I can do.
Edit: This is working for me. What do you think (I don't want to provide a patch because I've changed some more lines).
Removed hook_exit() and implemented hook_cron().
<?php
/**
* Implementation of hook_cron().
*/
function blog_reactions_cron() {
$limit = variable_get('blog_reactions_cron_quantity', 20);
$where = "'".implode("', '", variable_get('blog_reactions_node_types', array('blog')))."'";
$result = db_query_range("SELECT nid FROM {node} WHERE type IN(".$where.") ORDER BY changed ASC", 0, $limit);
while ($node = db_fetch_object($result)) {
$node = node_load(array('nid' => $node->nid));
if (($node->blog_reactions['last_update']+(60*60)) <= time()) {
$uri = url('node/'. $node->nid, NULL, NULL, TRUE);
$reactions = blog_reactions_fetch($node->nid, $uri);
blog_reactions_set_items($reactions);
watchdog('blog_reactions', t('Node @title (@type) was updated', array('@title' => $node->title, '@type' => $node->type)), WATCHDOG_NOTICE);
}
}
}
?>
Furthermore I've added a new field to blog_reactions_admin_settings().
<?php$form['blog_reactions_cron_quantity'] = array(
'#type' => 'select',
'#title' => t('Number of nodes per cron run'),
'#options' => drupal_map_assoc(array(10,20,30,40,50,75,100)),
'#default_value' => variable_get('blog_reactions_cron_quantity', 20),
'#description' => t('Choose the number of nodes wich will be updated per cron run. Default is 20.'),
);
?>
#9
I've thought about the cron and your comment
I think my code is not fulfilling all your requirements. I'm just checking for last node.changed. But I've seen that updating blog reactions has no impact on node.changed. Now I understand why we need an changed date. Can't we use the field blog_reactions.updated? If it doesn't exist or if the date is too long ago we have to run the blog_reactions_fetch() and blog_reactions_set_items() functions.
#10
<?php
/**
* Implementation of hook_cron().
*/
function blog_reactions_cron() {
$limit = variable_get('blog_reactions_cron_quantity', 20);
$types = variable_get('blog_reactions_node_types', array('blog'));
$where = implode("', '", $types);
$result = db_query_range("SELECT DISTINCT(n.nid), MAX(br.timestamp) as timestamp FROM {node} n
INNER JOIN {blog_reactions} br ON br.nid = n.nid
WHERE n.type IN('". $where ."')
GROUP BY n.nid
ORDER BY br.timestamp ASC", 0, $limit);
while ($row = db_fetch_object($result)) {
$uri = url('node/'. $row->nid, NULL, NULL, TRUE);
$reactions = blog_reactions_fetch($row->nid, $uri);
blog_reactions_set_items($reactions);
watchdog('blog_reactions', t('Node !nid has been updated', array('!nid' => $row->nid)), WATCHDOG_NOTICE, l('View', 'node/'. $row->nid));
}
}
?>
Does this work for you?
I removed node_load() it's too much of a overhead and changed the check for outdated nodes. I don't think we need the one hour timeout anymore, as the admin may decide when to run the cron.
#11
Your code is just great. Thanks a lot! But one problem is left. The query doesn't select new nodes, i.e. if there's no nid/timestamp in the table blog_reactions the cron won't check for blog reactions for this node. Is this a problem of the inner join? So what can we do to check also new nodes which haven't been checked yet?
Edit: I've changed "INNER JOIN" to "LEFT JOIN" and now four new nodes were checked and added to the blog_reactions table.
#12
LEFT JOIN works well.
The only problem is, that if there are too many nodes without reactions, we never update those with reactions.
New try:
<?php
/**
* Implementation of hook_cron().
*/
function blog_reactions_cron() {
$limit = variable_get('blog_reactions_cron_quantity', 20);
$types = variable_get('blog_reactions_node_types', array('blog'));
$last_update_nid = variable_get('blog_reactions_last_update_nid', 0);
$where = implode("', '", $types);
$result = db_query_range("SELECT DISTINCT(n.nid), MAX(br.timestamp) as timestamp FROM {node} n
LEFT JOIN {blog_reactions} br ON br.nid = n.nid
WHERE n.type IN('". $where ."')
AND n.nid > %d
GROUP BY n.nid
ORDER BY n.nid ASC", $last_update_nid, 0, $limit);
if (db_num_rows($result) == 0) {
variable_set('blog_reactions_last_update_nid', 0);
}
while ($row = db_fetch_object($result)) {
$uri = url('node/'. $row->nid, NULL, NULL, TRUE);
$reactions = blog_reactions_fetch($row->nid, $uri);
blog_reactions_set_items($reactions);
watchdog('blog_reactions', t('Node !nid has been updated', array('!nid' => $row->nid)), WATCHDOG_NOTICE, l('View', 'node/'. $row->nid));
variable_set('blog_reactions_last_update_nid', $row->nid);
}
}
?>
#13
Hi there,
I've tested your new code. Thanks a lot! I've set blog_reactions_cron_quantity to 5 nodes per cron and it worked very well. I'll let it run on my system and I'll inform you how it's working.
Awesome!
#14
Commited to 5,6,HEAD.
Thanks.
#15
Automatically closed -- issue fixed for two weeks with no activity.