Conflict in handling $unique between ActionFactory and DbStore

[davidarkemp]

ActionFactory has a fallback for when the Store doesn't implement save_unique_action, which includes the 'args' in the uniqueness.

https://github.com/woocommerce/action-scheduler/blob/628fd3ccf87649ba470240afa9abc4adb8bce7e0/classes/ActionScheduler_ActionFactory.php#L354C2-L377C3

However, DBStore (eventually) only uses the hook and the group for uniqueness in build_where_for_insert_clause:

https://github.com/woocommerce/action-scheduler/blob/628fd3ccf87649ba470240afa9abc4adb8bce7e0/classes/data-stores/ActionScheduler_DBStore.php#L185C2-L217C3

1. Is this conflict in behaviour intentional?
2. If not, which is the intended behaviour?

[RicherYang]

I also found this problem.
Hopefully, DBStore can be adjusted to accept args as find unique parameter.

[barryhughes]

However, DBStore (eventually) only uses the hook and the group for uniqueness in build_where_for_insert_clause

This was intentional.

Initially, args were going to be a part of how uniqueness was assessed but, before we released this functionality, that was changed in favor of a simpler model. There was however a thought that matching the args could be added as a facet of uniqueness at some later point.

ActionFactory has a fallback for when the Store doesn't implement save_unique_action, which includes the 'args' in the uniqueness.

I suspect this (the discrepancy between how uniqueness is assessed here, vs the DBStore) was an oversight. One should now be aligned with the other and, if doing so is viable, I think perhaps we should adopt a model where both matching strategies can be used (see this earlier comment).

[RicherYang]

As a plugin developer, the $unique is work with args or not is not the biggest question.

The most question is I can't real know after two function call what schedule action will added.
Because different data store ( DBStore and wpPostStore ) get different result.

as_schedule_single_action(time(), 'test_hook', [1], true);
as_schedule_single_action(time(), 'test_hook', [2], true);

It make $unique become a useless arg.

[barryhughes]

The most question is I can't real know.

For sure, the inconsistency you're describing is definitely problematic and should be resolved one way or another.

[x2764tech]

FWIW, as_has_scheduled_action does support $args, so you can use that to check for existing actions before scheduling a new one.

So @RicherYang's example might look like this:

if ( !as_has_scheduled_action( 'test_hook', [1] ) {
  as_schedule_single_action(time(), 'test_hook', [1]); //could also use as_enqueue_async_action here
}

[barryhughes]

That's definitely a decent fallback but just be aware of the potential pitfall, which is that it creates a race condition (and so you may end up with duplicate actions being scheduled). How likely it is that you will experience this of course depends on factors like how busy your site is, and how frequently that code will be invoked.