Skip to content

Commit

Permalink
Return true for as_next_scheduled() async actions
Browse files Browse the repository at this point in the history
The new async psuedo action type uses the NullSchedule to map to a
0000-00-00 00:00:00 date in MySQL and ensure it's always run as
soon as possible. However, that meant as_next_scheduled_action()
was returning boolean false for the next scheduled timestamp on
these, becuase their schedule()->next() value was NULL.

We need to return something other than false, because there are
pending actions.

There were a number of options for this return value:

1. int 0 to represent the absence of a schedule and/or that its
   scheduled for "the beginning of time" (which is the Unix Epoch in
   PHP's world). This is risky though, because it will be confused with
   false via weak comparison operators, so it's likely to create bugs
   with existing 3rd party code.
2. int 1, to represent a truthy value in int that is still near enough
    to the beginning of time to communicate the above messages.
3. time() to indicate that the next time any async action should be run
   is "now" (which is always true for any pending async action because
   they should be run ASAP). This could cause issues though, if 3rd party
   code is checking against as_next_scheduled_action() < time(), or for
   that matter, as_next_scheduled_action() > time().
4. NULL, to indicate there is a scheduled action (i.e. the return value
   is not false), but it doesn't have a scheduled timestamp. This return
   value has the same issue as 0.
5. return boolean true, to indicate there is a scheduled action, but it
   doesn't have a scheduled timestamp. This seems semantically correct,
   backward compatible and relatively robust. It will fail against any
   checks for as_next_scheduled_action() > time() though. I'm not sure
   if there is anyway around that though.
  • Loading branch information
thenbrent committed Jul 8, 2019
1 parent 706daa0 commit cacacea
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
4 changes: 3 additions & 1 deletion functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function as_unschedule_all_actions( $hook, $args = array(), $group = '' ) {
* @param array $args
* @param string $group
*
* @return int|bool The timestamp for the next occurrence, or false if nothing was found
* @return int|bool The timestamp for the next occurrence of a scheduled action, true for an async action or false if there is no matching, pending action.
*/
function as_next_scheduled_action( $hook, $args = NULL, $group = '' ) {
$params = array(
Expand All @@ -141,6 +141,8 @@ function as_next_scheduled_action( $hook, $args = NULL, $group = '' ) {
$next = $job->get_schedule()->next();
if ( $next ) {
return (int)($next->format('U'));
} elseif ( NULL === $next ) { // pending async action with NullSchedule
return true;
}
return false;
}
Expand Down
26 changes: 26 additions & 0 deletions tests/phpunit/procedural_api/procedural_api_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,32 @@ public function test_get_next() {
$this->assertEquals( $time->getTimestamp(), $next );
}

public function test_get_next_async() {
$hook = md5(rand());
$action_id = as_enqueue_async_action( $hook );

$next = as_next_scheduled_action( $hook );

$this->assertTrue( $next );

$store = ActionScheduler::store();

// Completed async actions should still return false
$store->mark_complete( $action_id );
$next = as_next_scheduled_action( $hook );
$this->assertFalse( $next );

// Failed async actions should still return false
$store->mark_failure( $action_id );
$next = as_next_scheduled_action( $hook );
$this->assertFalse( $next );

// Cancelled async actions should still return false
$store->cancel_action( $action_id );
$next = as_next_scheduled_action( $hook );
$this->assertFalse( $next );
}

public function provider_time_hook_args_group() {
$time = time() + 60 * 2;
$hook = md5( rand() );
Expand Down

0 comments on commit cacacea

Please sign in to comment.