Skip to content

Commit

Permalink
Scheduled Updates: Only validate schedule when logging (#36961)
Browse files Browse the repository at this point in the history 
* Scheduled Updates: Only validate schedule when logging

Removes all other schedule_exists checks to simplify clearing logs after a schedule was deleted. This will also help with moving non-schedule operations in the rest api endpoint to action callbacks in #36835.

* Update function docs
  • Loading branch information
@obenland
obenland authored Apr 22, 2024
1 parent 2e7b50a commit cb075b7
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 108 deletions.
3 changes: 1 addition & 2 deletions projects/packages/scheduled-updates/.phan/baseline.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
return [
// # Issue statistics:
// PhanUndeclaredProperty : 40+ occurrences
// PhanCompatibleAccessMethodOnTraitDefinition : 2 occurrences
// PhanPluginMixedKeyNoKey : 2 occurrences
// PhanRedundantCondition : 2 occurrences
// PhanTypeArraySuspiciousNullable : 2 occurrences
// PhanUndeclaredClassMethod : 2 occurrences
// PhanCompatibleAccessMethodOnTraitDefinition : 1 occurrence
// PhanNoopNew : 1 occurrence
// PhanTypeMismatchArgumentProbablyReal : 1 occurrence
// PhanUndeclaredFunction : 1 occurrence
Expand All @@ -24,7 +24,6 @@
'src/class-scheduled-updates.php' => ['PhanRedundantCondition', 'PhanTypeMismatchArgumentProbablyReal', 'PhanUndeclaredClassMethod'],
'src/pluggable.php' => ['PhanTypeArraySuspiciousNullable'],
'src/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-update-schedules.php' => ['PhanPluginMixedKeyNoKey', 'PhanUndeclaredFunction'],
'tests/php/class-scheduled-updates-logs-test.php' => ['PhanCompatibleAccessMethodOnTraitDefinition', 'PhanUndeclaredProperty'],
'tests/php/class-scheduled-updates-test.php' => ['PhanCompatibleAccessMethodOnTraitDefinition', 'PhanUndeclaredProperty'],
'tests/php/class-wpcom-rest-api-v2-endpoint-update-schedules-test.php' => ['PhanNoopNew'],
],
Expand Down
4 changes: 4 additions & 0 deletions projects/packages/scheduled-updates/changelog/remove-schedule-check
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Scheduled Updates: Removed checks for valid schedules when retreiving and clearing logs
79 changes: 25 additions & 54 deletions projects/packages/scheduled-updates/src/class-scheduled-updates-logs.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

namespace Automattic\Jetpack;

use WP_Error;

/**
* Scheduled_Update_Logs class
*
Expand Down Expand Up @@ -46,13 +44,18 @@ class Scheduled_Updates_Logs {
* @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
* @return bool True if the log was successfully saved, false otherwise.
*/
public static function log( $schedule_id, $action, $message = null, $context = null, $timestamp = null ) {
$events = wp_get_scheduled_events( Scheduled_Updates::PLUGIN_CRON_HOOK );
if ( ! isset( $events[ $schedule_id ] ) ) {
return false;
}

if ( null === $timestamp ) {
$timestamp = wp_date( 'U' );
}

$log_entry = array(
'timestamp' => intval( $timestamp ),
'action' => $action,
Expand All @@ -62,17 +65,13 @@ public static function log( $schedule_id, $action, $message = null, $context = n

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

if ( ! self::is_valid_schedule( $schedule_id ) ) {
return new WP_Error( 'invalid_schedule_id', 'Invalid schedule ID' );
}

if ( ! isset( $logs[ $schedule_id ] ) ) {
$logs[ $schedule_id ] = array();
}

$logs[ $schedule_id ][] = $log_entry;

// Keep only the logs for the last MAX_RUNS_PER_SCHEDULE runs per schedule_id
// Keep only the logs for the last MAX_RUNS_PER_SCHEDULE runs per schedule_id.
$start_count = 0;
$last_two_runs = array();
for ( $i = count( $logs[ $schedule_id ] ) - 1; $i >= 0; $i-- ) {
Expand All @@ -87,41 +86,38 @@ public static function log( $schedule_id, $action, $message = null, $context = n
$last_two_runs = array_reverse( $last_two_runs );
$logs[ $schedule_id ] = $last_two_runs;

update_option( self::OPTION_NAME, $logs );
return update_option( self::OPTION_NAME, $logs );
}

/**
* Retrieves the logs for a specific schedule_id or all logs if no schedule_id is provided.
*
* If a schedule_id is provided, the logs for that specific schedule are returned.
* If no schedule_id is provided, all logs are returned, with each schedule_id as a key in the array.
*
* @param string|null $schedule_id Optional. The ID of the schedule. If not provided, all logs will be returned.
* @return array {
* An array containing the logs, split by run.
* Each run is an array of log entries, where each log entry is an associative array containing the following keys:
*
* @return array|WP_Error
* An array containing the logs, split by run.
* If a schedule_id is provided, the logs for that specific schedule are returned.
* If no schedule_id is provided, all logs are returned, with each schedule_id as a key in the array.
* Each run is an array of log entries, where each log entry is an associative array
* containing the following keys:
* - 'timestamp' (int): The Unix timestamp of the log entry.
* - 'action' (string): The action constant representing the event.
* - 'message' (string|null): The message associated with the event, if available.
* - 'context' (mixed|null): Additional context data associated with the event, if available.
* @type int $timestamp The Unix timestamp of the log entry.
* @type string $action The action constant representing the event.
* @type string|null $message The message associated with the event, if available.
* @type mixed|null $context Additional context data associated with the event, if available.
* }
*/
public static function get( $schedule_id = null ) {
$logs = get_option( self::OPTION_NAME, array() );

if ( null === $schedule_id ) {
// Return all logs if no schedule_id is provided
// Return all logs if no schedule_id is provided.
$all_logs = array();
foreach ( $logs as $schedule_id => $schedule_logs ) {
$all_logs[ $schedule_id ] = self::split_logs_into_runs( $schedule_logs );
}
return $all_logs;
}

if ( ! self::is_valid_schedule( $schedule_id ) ) {
return new WP_Error( 'invalid_schedule_id', 'Invalid schedule ID' );
}

if ( ! isset( $logs[ $schedule_id ] ) ) {
return array();
}
Expand All @@ -134,25 +130,16 @@ public static function get( $schedule_id = null ) {
* Clears the logs for a specific schedule_id or all logs if no schedule_id is provided.
*
* @param string|null $schedule_id Optional. The ID of the schedule. If not provided, all logs will be cleared.
* @param bool $skip_validation Optional. Whether to skip the validation of the schedule_id.
*
* @return WP_Error|null
*/
public static function clear( string $schedule_id = null, $skip_validation = false ) {
public static function clear( string $schedule_id = null ) {
$logs = get_option( self::OPTION_NAME, array() );

if ( null === $schedule_id ) {
// Clear all logs if no schedule_id is provided
// Clear all logs if no schedule_id is provided.
$logs = array();
} else {
if ( ! $skip_validation && ! self::is_valid_schedule( $schedule_id ) ) {
return new WP_Error( 'invalid_schedule_id', 'Invalid schedule ID' );
}

if ( isset( $logs[ $schedule_id ] ) ) {
// Clear the logs for the specific schedule_id
unset( $logs[ $schedule_id ] );
}
// Clear the logs for the specific schedule_id.
unset( $logs[ $schedule_id ] );
}

update_option( self::OPTION_NAME, $logs );
Expand Down Expand Up @@ -239,22 +226,6 @@ private static function split_logs_into_runs( $logs ) {
return $runs;
}

/**
* Returns whether a schedule_id is valid.
*
* @param string $schedule_id The schedule id.
* @return bool
*/
private static function is_valid_schedule( $schedule_id ) {
$events = wp_get_scheduled_events( Scheduled_Updates::PLUGIN_CRON_HOOK );

if ( ! isset( $events[ $schedule_id ] ) ) {
return false;
}

return true;
}

/**
* Replaces the logs with the old schedule ID with new ones.
*
Expand Down
6 changes: 3 additions & 3 deletions ...heduled-updates/src/wpcom-endpoints/class-wpcom-rest-api-v2-endpoint-update-schedules.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,9 @@ public function add_log( $request ) {
$message = $request['message'];
$context = $request['context'];

$log = Scheduled_Updates_Logs::log( $schedule_id, $action, $message, $context );
$success = Scheduled_Updates_Logs::log( $schedule_id, $action, $message, $context );

if ( is_wp_error( $log ) ) {
if ( ! $success ) {
return new WP_Error( 'rest_invalid_schedule', __( 'The schedule could not be found.', 'jetpack-scheduled-updates' ), array( 'status' => 404 ) );
}

Expand Down Expand Up @@ -513,7 +513,7 @@ public function delete_item( $request, $clear_logs = true ) {

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

return rest_ensure_response( true );
Expand Down
Loading

0 comments on commit cb075b7

Please sign in to comment.