Skip to content

Commit

Permalink
Fix recurring schedules, e.g. Cron and Interval schedules
Browse files Browse the repository at this point in the history
Recurring schedules issues, as outlined in #330.

This patch fixes recurring schedules, and in the process, refactors
all schedules to avoid duplicate code and ambiguity of properties
and parameters.

This patch introduces to important new base classes:
 * ActionScheduler_Abstract_Schedule and
 * ActionScheduler_Abstract_RecurringSchedule

These are then used to implement all other schedule types - cron,
recurring/interval and simple. This helps unify the data on each
schedule. For example, schedules used to have different property
names referring to equivalent data.

For example, ActionScheduler_IntervalSchedule::start_timestamp
was the same as ActionScheduler_SimpleSchedule::timestamp.

Both of these timestamps actually referring to the date the action
was scheduled, but due to ambiguity in naming, like 'start' or 'date'
or 'first', the data was sometimes used to refer to the scheduled date,
and sometimes the first instance of a recurring schedule. While
unifying the properties, their nomenclature has also been changed
to avoid ambiguity.

Fixes #330 and associated issues - #256, #258, #291, #293 and #296.
  • Loading branch information
thenbrent committed Jul 31, 2019
1 parent 809a897 commit ae7d4c5
Show file tree
Hide file tree
Showing 26 changed files with 471 additions and 183 deletions.
43 changes: 42 additions & 1 deletion classes/ActionScheduler_ActionFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public function single( $hook, $args = array(), $when = null, $group = '' ) {
}

/**
* Create the first instance of an action recurring on a given interval.
*
* @param string $hook The hook to trigger when this action runs
* @param array $args Args to pass when the hook is triggered
* @param int $first Unix timestamp for the first run
Expand All @@ -100,8 +102,9 @@ public function recurring( $hook, $args = array(), $first = null, $interval = nu
return $this->store( $action );
}


/**
* Create the first instance of an action recurring on a Cron schedule.
*
* @param string $hook The hook to trigger when this action runs
* @param array $args Args to pass when the hook is triggered
* @param int $first Unix timestamp for the first run
Expand All @@ -121,6 +124,44 @@ public function cron( $hook, $args = array(), $first = null, $schedule = null, $
return $this->store( $action );
}

/**
* Create a successive instance of a recurring or cron action.
*
* Importantly, the action will be rescheduled to run based on the current date/time.
* That means when the action is scheduled to run in the past, the next scheduled date
* will be pushed forward. For example, if a recurring action set to run every hour
* was scheduled to run 5 seconds ago, it will be next scheduled for 1 hour in the
* future, which is 1 hour and 5 seconds from when it was last scheduled to run.
*
* Alternatively, if the action is scheduled to run in the future, and is run early,
* likely via manual intervention, then its schedule will change based on the time now.
* For example, if a recurring action set to run every day, and is run 12 hours early,
* it will run again in 24 hours, not 36 hours.
*
* This slippage is less of an issue with Cron actions, as the specific run time can
* be set for them to run, e.g. 1am each day. In those cases, and entire period would
* need to be missed before there was any change is scheduled, e.g. in the case of an
* action scheduled for 1am each day, the action would need to run an entire day late.
*
* @param ActionScheduler_Action $action The existing action.
*
* @return string The ID of the stored action
* @throws InvalidArgumentException If $action is not a recurring action.
*/
public function repeat( $action ) {
$schedule = $action->get_schedule();
$next = $schedule->get_next( as_get_datetime_object() );

if ( is_null( $next ) || ! $schedule->is_recurring() ) {
throw new InvalidArgumentException( __( 'Invalid action - must be a recurring action.', 'action-scheduler' ) );
}

$schedule_class = get_class( $schedule );
$new_schedule = new $schedule( $next, $schedule->get_recurrence(), $schedule->get_first_date() );
$new_action = new ActionScheduler_Action( $action->get_hook(), $action->get_args(), $new_schedule, $action->get_group() );
return $this->store( $new_action );
}

/**
* @param ActionScheduler_Action $action
*
Expand Down
20 changes: 10 additions & 10 deletions classes/ActionScheduler_ListTable.php
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,14 @@ private static function human_interval( $interval, $periods_to_include = 2 ) {
* @return string
*/
protected function get_recurrence( $action ) {
$recurrence = $action->get_schedule();
if ( $recurrence->is_recurring() ) {
if ( method_exists( $recurrence, 'interval_in_seconds' ) ) {
return sprintf( __( 'Every %s', 'action-scheduler' ), self::human_interval( $recurrence->interval_in_seconds() ) );
}
$schedule = $action->get_schedule();
if ( $schedule->is_recurring() ) {
$recurrence = $schedule->get_recurrence();

if ( method_exists( $recurrence, 'get_recurrence' ) ) {
return sprintf( __( 'Cron %s', 'action-scheduler' ), $recurrence->get_recurrence() );
if ( is_numeric( $recurrence ) ) {
return sprintf( __( 'Every %s', 'action-scheduler' ), self::human_interval( $recurrence ) );
} else {
return sprintf( __( '%s', 'action-scheduler' ), $recurrence );
}
}

Expand Down Expand Up @@ -395,13 +395,13 @@ protected function get_schedule_display_string( ActionScheduler_Schedule $schedu

$schedule_display_string = '';

if ( ! $schedule->get_start() ) {
if ( ! $schedule->get_date() ) {
return '0000-00-00 00:00:00';
}

$next_timestamp = $schedule->get_start()->getTimestamp();
$next_timestamp = $schedule->get_date()->getTimestamp();

$schedule_display_string .= $schedule->get_start()->format( 'Y-m-d H:i:s O' );
$schedule_display_string .= $schedule->get_date()->format( 'Y-m-d H:i:s O' );
$schedule_display_string .= '<br/>';

if ( gmdate( 'U' ) > $next_timestamp ) {
Expand Down
18 changes: 10 additions & 8 deletions classes/abstracts/ActionScheduler.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,16 @@ public static function init( $plugin_file ) {
*/
protected static function is_class_abstract( $class ) {
static $abstracts = array(
'ActionScheduler' => true,
'ActionScheduler_Abstract_ListTable' => true,
'ActionScheduler_Abstract_QueueRunner' => true,
'ActionScheduler_Lock' => true,
'ActionScheduler_Logger' => true,
'ActionScheduler_Abstract_Schema' => true,
'ActionScheduler_Store' => true,
'ActionScheduler_TimezoneHelper' => true,
'ActionScheduler' => true,
'ActionScheduler_Abstract_ListTable' => true,
'ActionScheduler_Abstract_QueueRunner' => true,
'ActionScheduler_Abstract_Schedule' => true,
'ActionScheduler_Abstract_RecurringSchedule' => true,
'ActionScheduler_Lock' => true,
'ActionScheduler_Logger' => true,
'ActionScheduler_Abstract_Schema' => true,
'ActionScheduler_Store' => true,
'ActionScheduler_TimezoneHelper' => true,
);

return isset( $abstracts[ $class ] ) && $abstracts[ $class ];
Expand Down
9 changes: 3 additions & 6 deletions classes/abstracts/ActionScheduler_Abstract_QueueRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,9 @@ public function process_action( $action_id, $context = '' ) {
* @param ActionScheduler_Action $action
*/
protected function schedule_next_instance( ActionScheduler_Action $action ) {
$schedule = $action->get_schedule();
$next = $schedule->next( as_get_datetime_object() );

if ( ! is_null( $next ) && $schedule->is_recurring() ) {
$schedule->set_next( $next );
$this->store->save_action( $action, $next );
try {
ActionScheduler::factory()->repeat( $action );
} catch ( Exception $e ) {
}
}

Expand Down
92 changes: 92 additions & 0 deletions classes/abstracts/ActionScheduler_Abstract_RecurringSchedule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?php

/**
* Class ActionScheduler_Abstract_RecurringSchedule
*/
abstract class ActionScheduler_Abstract_RecurringSchedule extends ActionScheduler_Abstract_Schedule {

/**
* The date & time the first instance of this schedule was setup to run (which may not be this instance).
*
* Schedule objects are attached to an action object. Each schedule stores the run date for that
* object as the start date - @see $this->start - and logic to calculate the next run date after
* that - @see $this->calculate_next(). The $first_date property also keeps a record of when the very
* first instance of this chain of schedules ran.
*
* @var DateTime
*/
private $first_date = NULL;

/**
* Timestamp equivalent of @see $this->first_date
*
* @var int
*/
protected $first_timestamp = 0;

/**
* The recurrance between each time an action is run using this schedule.
* Used to calculate the start date & time. Can be a number of seconds, in the
* case of ActionScheduler_IntervalSchedule, or a cron expression, as in the
* case of ActionScheduler_CronSchedule. Or something else.
*
* @var mixed
*/
protected $recurrence;

/**
* @param DateTime $date The date & time to run the action.
* @param mixed $recurrence The data used to determine the schedule's recurrance.
* @param DateTime|null $first (Optional) The date & time the first instance of this interval schedule ran. Default null, meaning this is the first instance.
*/
public function __construct( DateTime $date, $recurrence, DateTime $first = null ) {
parent::__construct( $date );
$this->first_date = empty( $first ) ? $date : $first;
$this->recurrence = $recurrence;
}

/**
* @return bool
*/
public function is_recurring() {
return true;
}

/**
* Get the date & time of the first schedule in this recurring series.
*
* @return DateTime|null
*/
public function get_first_date() {
return clone $this->first_date;
}

/**
* @return string
*/
public function get_recurrence() {
return $this->recurrence;
}

/**
* For PHP 5.2 compat, since DateTime objects can't be serialized
* @return array
*/
public function __sleep() {
$sleep_params = parent::__sleep();
$this->first_timestamp = $this->first_date->getTimestamp();
return array_merge( $sleep_params, array(
'first_timestamp',
'recurrence'
) );
}

public function __wakeup() {
parent::__wakeup();
if ( $this->first_timestamp > 0 ) {
$this->first_date = as_get_datetime_object( $this->first_timestamp );
} else {
$this->first_date = $this->get_date();
}
}
}
82 changes: 82 additions & 0 deletions classes/abstracts/ActionScheduler_Abstract_Schedule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

/**
* Class ActionScheduler_Abstract_Schedule
*/
abstract class ActionScheduler_Abstract_Schedule extends ActionScheduler_Schedule_Deprecated {

/**
* The date & time the schedule is set to run.
*
* @var DateTime
*/
private $scheduled_date = NULL;

/**
* Timestamp equivalent of @see $this->scheduled_date
*
* @var int
*/
protected $scheduled_timestamp = 0;

/**
* @param DateTime $date The date & time to run the action.
*/
public function __construct( DateTime $date ) {
$this->scheduled_date = $date;
}

/**
* Check if a schedule should recur.
*
* @return bool
*/
abstract public function is_recurring();

/**
* Calculate when the next instance of this schedule would run based on a given date & time.
*
* @param DateTime $after
* @return DateTime
*/
abstract protected function calculate_next( DateTime $after );

/**
* Get the next date & time when this schedule should run after a given date & time.
*
* @param DateTime $after
* @return DateTime|null
*/
public function get_next( DateTime $after ) {
$after = clone $after;
if ( $after > $this->scheduled_date ) {
$after = $this->calculate_next( $after );
return $after;
}
return clone $this->scheduled_date;
}

/**
* Get the date & time the schedule is set to run.
*
* @return DateTime|null
*/
public function get_date() {
return $this->scheduled_date;
}

/**
* For PHP 5.2 compat, since DateTime objects can't be serialized
* @return array
*/
public function __sleep() {
$this->scheduled_timestamp = $this->scheduled_date->getTimestamp();
return array(
'scheduled_timestamp',
);
}

public function __wakeup() {
$this->scheduled_date = as_get_datetime_object( $this->scheduled_timestamp );
}
}
4 changes: 2 additions & 2 deletions classes/abstracts/ActionScheduler_Store.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ protected function validate_sql_comparator( $comparison_operator ) {
* @return string
*/
protected function get_scheduled_date_string( ActionScheduler_Action $action, DateTime $scheduled_date = NULL ) {
$next = null === $scheduled_date ? $action->get_schedule()->next() : $scheduled_date;
$next = null === $scheduled_date ? $action->get_schedule()->get_date() : $scheduled_date;
if ( ! $next ) {
return '0000-00-00 00:00:00';
}
Expand All @@ -165,7 +165,7 @@ protected function get_scheduled_date_string( ActionScheduler_Action $action, Da
* @return string
*/
protected function get_scheduled_date_string_local( ActionScheduler_Action $action, DateTime $scheduled_date = NULL ) {
$next = null === $scheduled_date ? $action->get_schedule()->next() : $scheduled_date;
$next = null === $scheduled_date ? $action->get_schedule()->get_date() : $scheduled_date;
if ( ! $next ) {
return '0000-00-00 00:00:00';
}
Expand Down
4 changes: 3 additions & 1 deletion classes/actions/ActionScheduler_CanceledAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class ActionScheduler_CanceledAction extends ActionScheduler_FinishedAction {
*/
public function __construct( $hook, array $args = array(), ActionScheduler_Schedule $schedule = null, $group = '' ) {
parent::__construct( $hook, $args, $schedule, $group );
$this->set_schedule( new ActionScheduler_NullSchedule() );
if ( is_null( $schedule ) ) {
$this->set_schedule( new ActionScheduler_NullSchedule() );
}
}
}
4 changes: 0 additions & 4 deletions classes/data-stores/ActionScheduler_wpPostStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ public function save_action( ActionScheduler_Action $action, DateTime $scheduled
$post_id = $this->save_post_array( $post_array );
$schedule = $action->get_schedule();

if ( ! is_null( $scheduled_date ) && $schedule->is_recurring() ) {
$schedule = new ActionScheduler_IntervalSchedule( $scheduled_date, $schedule->interval_in_seconds() );
}

$this->save_post_schedule( $post_id, $schedule );
$this->save_action_group( $post_id, $action->get_group() );
do_action( 'action_scheduler_stored_action', $post_id );
Expand Down
2 changes: 1 addition & 1 deletion classes/migration/ActionMigrator.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function migrate( $source_action_id ) {
$status = '';
}

if ( empty( $status ) || ! $action->get_schedule()->next() ) {
if ( empty( $status ) || ! $action->get_schedule()->get_date() ) {
// empty status means the action didn't exist
// null schedule means it's missing vital data
// delete it and move on
Expand Down
Loading

0 comments on commit ae7d4c5

Please sign in to comment.