Skip to content

Commit

Permalink
Scheduled Updates: Read status from logs directly (#36981)
Browse files Browse the repository at this point in the history
* Add new replace logs schedule id function

* Remove set_scheduled_update_status call

* Remove status endpoint tests

* Remove set_scheduled_update_status call

* Expand log function

* Add schedule id check

* Update unit test

* Add params to delete_item for clear logs

* get logs from option directly

* changelog

* Update get_latest_status function
  • Loading branch information
ouikhuan authored Apr 19, 2024
1 parent 06f4893 commit 26bfa7d
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 108 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: changed

Scheduled Updates: Change how we read status and get it from logs directly
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,14 @@ class Scheduled_Updates_Logs {
* @param string $action The action constant representing the event.
* @param string $message Optional. The message associated with the event.
* @param mixed $context Optional. Additional context data associated with the event.
* @param int $timestamp Optional. The Unix timestamp of the log entry. Default is the current time.
*
* @return WP_Error|null
*/
public static function log( $schedule_id, $action, $message = null, $context = null ) {
$timestamp = wp_date( 'U' );
public static function log( $schedule_id, $action, $message = null, $context = null, $timestamp = null ) {
if ( null === $timestamp ) {
$timestamp = wp_date( 'U' );
}
$log_entry = array(
'timestamp' => intval( $timestamp ),
'action' => $action,
Expand Down Expand Up @@ -251,4 +254,33 @@ private static function is_valid_schedule( $schedule_id ) {

return true;
}

/**
* Replaces the logs with the old schedule ID with new ones.
*
* @param string $old_schedule_id The old schedule ID.
* @param string $new_schedule_id The new schedule ID.
*
* @return bool True if the logs were successfully replaced, false otherwise.
*/
public static function replace_logs_schedule_id( $old_schedule_id, $new_schedule_id ) {

if ( $old_schedule_id === $new_schedule_id ) {
return false;
}

$logs = get_option( self::OPTION_NAME, array() );

if ( isset( $logs[ $old_schedule_id ] ) ) {
// Replace the logs with the old schedule ID with new ones
$logs[ $new_schedule_id ] = $logs[ $old_schedule_id ];
unset( $logs[ $old_schedule_id ] );

update_option( self::OPTION_NAME, $logs );

return true;
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ public static function run_scheduled_update( ...$plugins ) {
$plugins_to_update = array_intersect_key( $plugins_to_update, array_flip( $plugins ) );

if ( empty( $plugins_to_update ) ) {
// No updates available. Update the status to 'success' and return.
self::set_scheduled_update_status( $schedule_id, time(), 'success' );

// Log a start and success.
Scheduled_Updates_Logs::log(
Expand Down Expand Up @@ -222,16 +220,10 @@ public static function set_scheduled_update_status( $schedule_id, $timestamp, $s
* Get the last status of a scheduled update.
*
* @param string $schedule_id Request ID.
* @return array|null Last status of the scheduled update or null if not found.
* @return array|false Last status of the scheduled update or false if not found.
*/
public static function get_scheduled_update_status( $schedule_id ) {
$status = Scheduled_Updates_Logs::infer_status_from_logs( $schedule_id );
if ( false !== $status ) {
return $status;
}

$statuses = get_option( 'jetpack_scheduled_update_statuses', array() );
return $statuses[ $schedule_id ] ?? null;
return Scheduled_Updates_Logs::infer_status_from_logs( $schedule_id );
}

/**
Expand Down Expand Up @@ -408,11 +400,7 @@ public static function deleted_plugin( $plugin_file, $deleted ) {

// Inherit the status from the previous schedule.
if ( $status ) {
self::set_scheduled_update_status(
$schedule_id,
$status['last_run_timestamp'],
$status['last_run_status']
);
Scheduled_Updates_Logs::replace_logs_schedule_id( $id, $schedule_id );
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,6 @@ public function create_item( $request ) {

$id = Scheduled_Updates::generate_schedule_id( $plugins );

// Set an empty status of a schedule on creation/modify.
Scheduled_Updates::set_scheduled_update_status( $id, null, null );

return rest_ensure_response( $id );
}

Expand Down Expand Up @@ -347,7 +344,8 @@ public function update_item( $request ) {

$previous_schedule_status = Scheduled_Updates::get_scheduled_update_status( $request['schedule_id'] );

$deleted = $this->delete_item( $request );
$clear_logs = false;
$deleted = $this->delete_item( $request, $clear_logs );
if ( is_wp_error( $deleted ) ) {
return $deleted;
}
Expand All @@ -356,7 +354,7 @@ public function update_item( $request ) {

// Sets the previous status.
if ( $previous_schedule_status ) {
Scheduled_Updates::set_scheduled_update_status( $item->data, $previous_schedule_status['last_run_timestamp'], $previous_schedule_status['last_run_status'] );
Scheduled_Updates_Logs::replace_logs_schedule_id( $request['schedule_id'], $item->data );
}

return $item;
Expand Down Expand Up @@ -474,9 +472,10 @@ public function delete_item_permissions_check( $request ) { // phpcs:ignore Vari
* Deletes an existing update schedule.
*
* @param WP_REST_Request $request Request object.
* @param bool $clear_logs Whether to clear the logs or not.
* @return WP_REST_Response|WP_Error
*/
public function delete_item( $request ) {
public function delete_item( $request, $clear_logs = true ) {
$events = wp_get_scheduled_events( Scheduled_Updates::PLUGIN_CRON_HOOK );

if ( ! isset( $events[ $request['schedule_id'] ] ) ) {
Expand Down Expand Up @@ -513,7 +512,9 @@ public function delete_item( $request ) {
}

// Delete logs
Scheduled_Updates_Logs::clear( $request['schedule_id'], true );
if ( $clear_logs ) {
Scheduled_Updates_Logs::clear( $request['schedule_id'], true );
}

return rest_ensure_response( true );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,87 +441,6 @@ public function test_empty_last_run() {
);
}

/**
* Update event status.
*
* @covers ::update_status
*/
public function test_update_event_status() {
$plugins = array(
'custom-plugin/custom-plugin.php',
'gutenberg/gutenberg.php',
);
$id_1 = Scheduled_Updates::generate_schedule_id( $plugins );
$body = array(
'last_run_timestamp' => 1,
'last_run_status' => 'success',
);

wp_schedule_event( strtotime( 'next Tuesday 0:00' ), 'daily', Scheduled_Updates::PLUGIN_CRON_HOOK, $plugins );

$request = new WP_REST_Request( 'POST', '/wpcom/v2/update-schedules/' . $id_1 . '/status' );
$request->set_body_params( $body );

wp_set_current_user( $this->admin_id );

$request = new WP_REST_Request( 'POST', '/wpcom/v2/update-schedules/abc/status' );
$request->set_body_params( $body );
$result = rest_do_request( $request );

$this->assertSame( 404, $result->get_status() );

$request = new WP_REST_Request( 'POST', '/wpcom/v2/update-schedules/' . $id_1 . '/status' );
$request->set_body_params( $body );
$result = rest_do_request( $request );

$this->assertSame( 200, $result->get_status() );
$this->assertSame( 1, $result->get_data()['last_run_timestamp'] );
$this->assertSame( 'success', $result->get_data()['last_run_status'] );

$request = new WP_REST_Request( 'GET', '/wpcom/v2/update-schedules' );
$result = rest_do_request( $request );

$this->assertSame( 200, $result->get_status() );

$events = $result->get_data();

$this->assertIsArray( $events );
$this->assertArrayHasKey( $id_1, $events );
$this->assertSame( 1, $events[ $id_1 ]['last_run_timestamp'] );
$this->assertSame( 'success', $events[ $id_1 ]['last_run_status'] );

$plugins = array(
'hello-dolly/hello.php',
);
$id_2 = Scheduled_Updates::generate_schedule_id( $plugins );
$body = array(
'last_run_timestamp' => 2,
'last_run_status' => 'failure-and-rollback',
);

wp_schedule_event( strtotime( 'next Tuesday 09:00' ), 'daily', Scheduled_Updates::PLUGIN_CRON_HOOK, $plugins );

$request = new WP_REST_Request( 'POST', '/wpcom/v2/update-schedules/' . $id_2 . '/status' );
$request->set_body_params( $body );
$result = rest_do_request( $request );

$this->assertSame( 200, $result->get_status() );

$request = new WP_REST_Request( 'GET', '/wpcom/v2/update-schedules' );
$result = rest_do_request( $request );

$this->assertSame( 200, $result->get_status() );

$events = $result->get_data();

$this->assertArrayHasKey( $id_1, $events );
$this->assertSame( 1, $events[ $id_1 ]['last_run_timestamp'] );
$this->assertArrayHasKey( $id_2, $events );
$this->assertSame( 2, $events[ $id_2 ]['last_run_timestamp'] );
$this->assertSame( 'success', $events[ $id_1 ]['last_run_status'] );
$this->assertSame( 'failure-and-rollback', $events[ $id_2 ]['last_run_status'] );
}

/**
* Include over 10 plugins when creating a schedule.
*
Expand Down Expand Up @@ -688,7 +607,21 @@ public function test_update_item_with_status() {

wp_schedule_event( strtotime( 'next Monday 8:00' ), 'weekly', Scheduled_Updates::PLUGIN_CRON_HOOK, $plugins );

Scheduled_Updates::set_scheduled_update_status( $schedule_id, $timestamp, $status );
// Log a start and success.
Scheduled_Updates_Logs::log(
$schedule_id,
Scheduled_Updates_Logs::PLUGIN_UPDATES_START,
'no_plugins_to_update',
null,
$timestamp
);
Scheduled_Updates_Logs::log(
$schedule_id,
Scheduled_Updates_Logs::PLUGIN_UPDATES_SUCCESS,
'no_plugins_to_update',
null,
$timestamp
);

$request = new WP_REST_Request( 'PUT', '/wpcom/v2/update-schedules/' . $schedule_id );
$request->set_body_params(
Expand All @@ -710,8 +643,8 @@ public function test_update_item_with_status() {

// Get the updated status
$updated_status = Scheduled_Updates::get_scheduled_update_status( $schedule_id );
if ( $updated_status === null ) {
$this->fail( 'Scheduled_Updates::get_scheduled_update_status() returned null.' );
if ( $updated_status === false ) {
$this->fail( 'Scheduled_Updates::get_scheduled_update_status() returned false.' );
} else {
$this->assertIsArray( $updated_status, 'Scheduled_Updates::get_scheduled_update_status() should return an array.' );
// doing these null checks for the static analyzer
Expand Down

0 comments on commit 26bfa7d

Please sign in to comment.