From cacaceaf58a3cfaab768b9460328a2ab6ff0e3eb Mon Sep 17 00:00:00 2001 From: Brent Shepherd Date: Fri, 5 Jul 2019 13:42:51 +1000 Subject: [PATCH] Return true for as_next_scheduled() async actions 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. --- functions.php | 4 ++- .../procedural_api/procedural_api_Test.php | 26 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/functions.php b/functions.php index 732cd538a..e67764ae3 100644 --- a/functions.php +++ b/functions.php @@ -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( @@ -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; } diff --git a/tests/phpunit/procedural_api/procedural_api_Test.php b/tests/phpunit/procedural_api/procedural_api_Test.php index 543add798..c54925955 100644 --- a/tests/phpunit/procedural_api/procedural_api_Test.php +++ b/tests/phpunit/procedural_api/procedural_api_Test.php @@ -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() );