CA predicates maybe don't need to be prevented from being evaluated more than once.

This won't keep a predicate from running over the same entity if it's modified, will it? It seems to just take a snapshot of the arguments, so if one changed the key won't match later, correct?

I think you're right though. We should discuss the drawbacks of not locking and go from there... One I can think of would be potential for infinite recursion... but I don't know if that alone justifies it. With power comes responsibility, ya know?

I'm starting to think this locking business is going to cause us a lot of problems. I realized this problem is tied closely to this issue: http://www.ubercart.org/forum/support/9293/file_product_not_asigned_user... and, also, I'm thinking this one: http://drupal.org/node/331667

We need to determine what path to take on this one and figure out a way to work around the locking if we end up considering it necessary. My feeling is that we should remove the locks... they afford a little performance gain, but in the case of these issues, maybe a bit of premature optimization as well.

I have just run into this problem as well.

The patch above seems to make more sense then what is currently in the code. Ie: you should be able to perform the same action on different sets of arguments.

As far as I can see the thing that needs to be avoided here is the case where the exact some CA is evaluated twice, things like the same email getting sent to a user more then one.

But if there is going to be locking then it needs to lock on both the predicates and what its acting on.

This is still locking in ways that I wouldn't expect.

Please see this snippet of code from UC Product Triggers module.

/**
 * Implementation of hook_cart_itm().
 */
function uc_product_triggers_cart_item($op, &$item) {
  if ($op == 'remove') {
    $node = node_load($item->nid);
    ca_pull_trigger('uc_product_triggers_cart_remove_item', $node);
  }
}

The idea here is to pair this with something like the CA Taxonomy module, and then be able to act on groups of nodes added or removed from the shopping cart. However this doesn't work as the locking makes it process on the first trigger pull only, and not subsequent calls to ca_trigger_pull().

I was using the patch from http://www.ubercart.org/comment/34894/Re_Hey_Neil_Are_you_saying to make this work better, but I'd like to find a better solution. (And I'd really like to get this into ubercart before release)

I would like to state again that I think we should discuss in a little more depth the possibility of ditching the locks completely.

Some options here:

Option 1

function ca_pull_trigger($trigger, $args, $lock = FALSE) {

Where $args is an array of all the arguments to be evaluated, and $lock could be passed to ca_load_trigger_predicates().

Ideally we could keep them if there was a way to selectively disable them.

Option 2 (ugly)

Alternatively until then, modules like UC Product Triggers could implement their own version of ca_pull_trigger() and ca_load_trigger_predicates().

(ca_pull_trigger_unlocked() or ca_load_trigger_predicates_unlocked() or some such )

Option 3 (ugly)

Add a $reset param to ca_load_trigger_predicates().

Nevermind, I need to check the latest releases more often. I was still looking at beta6.