I was programatically adding nodes to a queue with code something like this:

$qid = $x; $sqid = $y;
foreach (items() as $nid) {
  $q = nodequeue_load($qid);
  $sq = nodequeue_load_subqueue($sqid);
  nodequeue_subqueue_add($q, $sq, $nid);
}

This added nodes to the queue, but all with the same position (because the subqueue was cached and therefore its count was not updated). There should probably be an argument to nodequeue_load_subqueue that allows you to refresh the cache.

I got around this by putting this line before the call to nodequeue_subqueue_add:

 $sq->count = db_result(db_query("SELECT COUNT(*) FROM {nodequeue_nodes} WHERE qid=%d AND sqid=%d", $qid, $sqid));
CommentFileSizeAuthor
#10 321866.patch5.84 KBezra-g
#6 321866-nodequeue-cache-bypass.patch1.38 KBandrewlevine

Comments

ezra-g’s picture

ezra-g commented

Good idea. Are you able to roll a patch that would add a $bypass_cache parameter to all the load_queue and load_subqueue functions? That's something I would definitely accept.

ezra-g’s picture

ezra-g commented
Title: caching of subqueues causes failure of nodequeue_subqueue_add » API Improvement: caching of subqueues causes failure of nodequeue_subqueue_add
dergachev’s picture

dergachev commented

+1. Just ran into this very bug by using views_bulk_operations, which supports nodequeue through the actions that nodequeue exposes. Perhaps it would also make sense to have these exposed actions use the new $bypass_cache flag too?

Obviously this applies to D6 too

dergachev’s picture

dergachev commented
Version: 5.x-2.2 » 7.x-2.x-dev
Priority: Normal » Critical
Status: Active » Needs review

The surely isn't a proper way of doing it, but it works well for me. Basically incorporates original poster's suggestion.

Index: site/sites/all/modules/nodequeue/nodequeue.module
===================================================================
--- site/sites/all/modules/nodequeue/nodequeue.module   (revision 1830)
+++ site/sites/all/modules/nodequeue/nodequeue.module   (working copy)
@@ -2044,7 +2044,9 @@
     // 0 means infinity so never do this if false
     nodequeue_check_subqueue_size($queue, $subqueue, $queue->size - 1);
   }
-
+       //EW_HACK: ensuring that count is updated to deal with caching bug as outlined in #321866
+  $subqueue->count = db_result(db_query("SELECT COUNT(*) FROM {nodequeue_nodes} WHERE qid=%d AND sqid=%d", $queue->qid, $subqueue->sqid));
+       //END_OF_HACK
   db_query("INSERT INTO {nodequeue_nodes} (sqid, qid, nid, position, timestamp) VALUES (%d, %d, %d, %d, %d)", $subqueue->sqid, $queue->qid, $nid, $subqueue->count + 1, time());
 }
dergachev’s picture

dergachev commented
Title: API Improvement: caching of subqueues causes failure of nodequeue_subqueue_add » nodequeue_add_action fails when importing multiple nodes in 1 request

I think the title is misleading. This is definitely a bug, since there's a glaring inconsistency between the caching behavior of nodequeue_load_subqueue, the assumptions made in nodequeue_subqueue_add, and in turn the resulting behavior of nodequeue_add_action.

andrewlevine’s picture

andrewlevine commented
Title: nodequeue_add_action fails when importing multiple nodes in 1 request » API Improvement: caching of subqueues causes failure of nodequeue_subqueue_add
Version: 7.x-2.x-dev » 5.x-2.x-dev
StatusFileSize
new 321866-nodequeue-cache-bypass.patch1.38 KB

Not sure about the action bug that evolving web is talking about, but here is a patch for the $bypass_cache addition ezra-g was talking about in #1

ezra-g’s picture

ezra-g commented
Status: Needs review » Fixed

Fixed. Thanks!

I committed rev 1.65 just as db1.drupal.org failed, so the commit doesn't show up on this project's list of commits.

dergachev’s picture

dergachev commented
Status: Fixed » Active

The nodequeue module exposes two actions via hook_action_info that add and remove nodes to a queue.
If these actions don't use the new API as defined in this patch, they're not going to fail if they're done repeatedly in the same request.

See nodequeue.actions.inc

I'd roll a patch but my hack above is working for me.

ezra-g’s picture

ezra-g commented

@evolvingweb: Could you please specifically point to the parts of the code in the Actions integration that need to be modified?

Thanks!

ezra-g’s picture

ezra-g commented
Status: Active » Needs review
StatusFileSize
new 321866.patch5.84 KB

This patch builds on what's already been committed and adds cache prevention in more functions that load queues/subqueues, and aims to fix the potential actions issues eluded to by evolvingweb.

ezra-g’s picture

ezra-g commented
Status: Needs review » Fixed

I committed this to D5 and D6.

Status: Fixed » Closed (fixed)

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