diff --git a/.github/changelog/3138-from-description b/.github/changelog/3138-from-description new file mode 100644 index 0000000000..16f0a5198b --- /dev/null +++ b/.github/changelog/3138-from-description @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Fix cleanup jobs silently doing nothing on sites where purge retention options were not set. diff --git a/includes/class-options.php b/includes/class-options.php index 7fbe3a9162..b982098f13 100644 --- a/includes/class-options.php +++ b/includes/class-options.php @@ -35,6 +35,10 @@ public static function init() { \add_filter( 'option_activitypub_support_post_types', array( self::class, 'support_post_types_ensure_array' ) ); \add_filter( 'option_activitypub_object_type', array( self::class, 'default_object_type' ) ); + \add_filter( 'option_activitypub_outbox_purge_days', array( self::class, 'sanitize_purge_days' ) ); + \add_filter( 'option_activitypub_inbox_purge_days', array( self::class, 'sanitize_purge_days' ) ); + \add_filter( 'option_activitypub_ap_post_purge_days', array( self::class, 'sanitize_purge_days' ) ); + \add_action( 'update_option_activitypub_relay_mode', array( self::class, 'relay_mode_changed' ), 10, 2 ); } @@ -226,9 +230,12 @@ public static function register_settings() { 'activitypub_advanced', 'activitypub_outbox_purge_days', array( - 'type' => 'integer', - 'description' => 'Number of days to keep items in the Outbox.', - 'default' => 180, + 'type' => 'integer', + 'description' => 'Number of days to keep items in the Outbox.', + 'default' => ACTIVITYPUB_OUTBOX_PURGE_DAYS, + 'sanitize_callback' => static function ( $value ) { + return \max( 1, \absint( $value ) ); + }, ) ); @@ -236,9 +243,12 @@ public static function register_settings() { 'activitypub_advanced', 'activitypub_inbox_purge_days', array( - 'type' => 'integer', - 'description' => 'Number of days to keep items in the Inbox.', - 'default' => 180, + 'type' => 'integer', + 'description' => 'Number of days to keep items in the Inbox.', + 'default' => ACTIVITYPUB_INBOX_PURGE_DAYS, + 'sanitize_callback' => static function ( $value ) { + return \max( 1, \absint( $value ) ); + }, ) ); @@ -246,9 +256,12 @@ public static function register_settings() { 'activitypub_advanced', 'activitypub_ap_post_purge_days', array( - 'type' => 'integer', - 'description' => 'Number of days to keep remote posts.', - 'default' => 30, + 'type' => 'integer', + 'description' => 'Number of days to keep remote posts.', + 'default' => ACTIVITYPUB_AP_POST_PURGE_DAYS, + 'sanitize_callback' => static function ( $value ) { + return \max( 1, \absint( $value ) ); + }, ) ); @@ -660,6 +673,34 @@ public static function default_object_type( $value ) { return $value; } + /** + * Sanitize purge day values. + * + * Ensures the value is a non-negative integer. Returns the + * registered default when the stored value is empty or false + * (option not properly set), but allows 0 to disable purging. + * + * @since unreleased + * + * @param mixed $value The stored option value. + * + * @return int The sanitized value. + */ + public static function sanitize_purge_days( $value ) { + if ( '' === $value || false === $value ) { + $filter = \current_filter(); + $defaults = array( + 'option_activitypub_outbox_purge_days' => ACTIVITYPUB_OUTBOX_PURGE_DAYS, + 'option_activitypub_inbox_purge_days' => ACTIVITYPUB_INBOX_PURGE_DAYS, + 'option_activitypub_ap_post_purge_days' => ACTIVITYPUB_AP_POST_PURGE_DAYS, + ); + + return $defaults[ $filter ] ?? ACTIVITYPUB_OUTBOX_PURGE_DAYS; + } + + return \max( 1, \absint( $value ) ); + } + /** * Handle relay mode option changes. * diff --git a/includes/class-scheduler.php b/includes/class-scheduler.php index cd236a4b5b..1ec2ade8f2 100644 --- a/includes/class-scheduler.php +++ b/includes/class-scheduler.php @@ -387,24 +387,21 @@ public static function reprocess_outbox() { * Purge outbox items based on a schedule. */ public static function purge_outbox() { - $days = (int) \get_option( 'activitypub_outbox_purge_days', 180 ); - Outbox::purge( $days ); + Outbox::purge( \get_option( 'activitypub_outbox_purge_days', ACTIVITYPUB_OUTBOX_PURGE_DAYS ) ); } /** * Purge inbox items based on a schedule. */ public static function purge_inbox() { - $days = (int) \get_option( 'activitypub_inbox_purge_days', 180 ); - Inbox::purge( $days ); + Inbox::purge( \get_option( 'activitypub_inbox_purge_days', ACTIVITYPUB_INBOX_PURGE_DAYS ) ); } /** * Purge remote posts based on a schedule. */ public static function purge_ap_posts() { - $days = (int) \get_option( 'activitypub_ap_post_purge_days', 30 ); - Remote_Posts::purge( $days ); + Remote_Posts::purge( \get_option( 'activitypub_ap_post_purge_days', ACTIVITYPUB_AP_POST_PURGE_DAYS ) ); } /** diff --git a/includes/constants.php b/includes/constants.php index 8c192d511b..2635d79f7a 100644 --- a/includes/constants.php +++ b/includes/constants.php @@ -109,3 +109,8 @@ * @see https://github.com/tfredrich/RestApiTutorial.com/blob/master/content/advanced/responses/retries.md */ define( 'ACTIVITYPUB_RETRY_ERROR_CODES', array( 408, 429, 500, 502, 503, 504 ) ); + +// Default purge retention periods (in days). +define( 'ACTIVITYPUB_OUTBOX_PURGE_DAYS', 180 ); +define( 'ACTIVITYPUB_INBOX_PURGE_DAYS', 180 ); +define( 'ACTIVITYPUB_AP_POST_PURGE_DAYS', 30 ); diff --git a/includes/wp-admin/class-health-check.php b/includes/wp-admin/class-health-check.php index b96c76a0eb..a77184a021 100644 --- a/includes/wp-admin/class-health-check.php +++ b/includes/wp-admin/class-health-check.php @@ -386,13 +386,13 @@ public static function debug_information( $info ) { $info['activitypub']['fields']['activitypub_outbox_purge_days'] = array( 'label' => \__( 'Outbox Retention Period', 'activitypub' ), - 'value' => \esc_attr( (int) \get_option( 'activitypub_outbox_purge_days', 180 ) ), + 'value' => \esc_attr( (int) \get_option( 'activitypub_outbox_purge_days', ACTIVITYPUB_OUTBOX_PURGE_DAYS ) ), 'private' => false, ); $info['activitypub']['fields']['activitypub_ap_post_purge_days'] = array( 'label' => \__( 'Remote Posts Retention Period', 'activitypub' ), - 'value' => \esc_attr( (int) \get_option( 'activitypub_ap_post_purge_days', 30 ) ), + 'value' => \esc_attr( (int) \get_option( 'activitypub_ap_post_purge_days', ACTIVITYPUB_AP_POST_PURGE_DAYS ) ), 'private' => false, ); diff --git a/tests/phpunit/tests/includes/class-test-options.php b/tests/phpunit/tests/includes/class-test-options.php index 61279267d2..b97fc78b76 100644 --- a/tests/phpunit/tests/includes/class-test-options.php +++ b/tests/phpunit/tests/includes/class-test-options.php @@ -242,4 +242,56 @@ public function test_default_quote_policy_sanitizes_invalid_values() { \update_option( 'activitypub_default_quote_policy', '' ); $this->assertEquals( ACTIVITYPUB_INTERACTION_POLICY_ANYONE, \get_option( 'activitypub_default_quote_policy' ) ); } + + /** + * Test purge days returns default when option is not set. + * + * @covers \Activitypub\Options::sanitize_purge_days + */ + public function test_purge_days_returns_default_when_unset() { + \delete_option( 'activitypub_outbox_purge_days' ); + + $this->assertEquals( + ACTIVITYPUB_OUTBOX_PURGE_DAYS, + \get_option( 'activitypub_outbox_purge_days', ACTIVITYPUB_OUTBOX_PURGE_DAYS ) + ); + } + + /** + * Test purge days does not allow zero. + * + * @covers \Activitypub\Options::sanitize_purge_days + */ + public function test_purge_days_does_not_allow_zero() { + Options::register_settings(); + + \update_option( 'activitypub_outbox_purge_days', 0 ); + $this->assertGreaterThanOrEqual( 1, \get_option( 'activitypub_outbox_purge_days' ) ); + } + + /** + * Test purge days returns default when stored value is empty string. + * + * @covers \Activitypub\Options::sanitize_purge_days + */ + public function test_purge_days_returns_default_for_empty_string() { + \update_option( 'activitypub_outbox_purge_days', '' ); + + $this->assertEquals( + ACTIVITYPUB_OUTBOX_PURGE_DAYS, + \get_option( 'activitypub_outbox_purge_days' ) + ); + } + + /** + * Test purge days sanitizes negative values. + * + * @covers \Activitypub\Options::sanitize_purge_days + */ + public function test_purge_days_sanitizes_negative() { + Options::register_settings(); + + \update_option( 'activitypub_outbox_purge_days', -5 ); + $this->assertGreaterThanOrEqual( 1, \get_option( 'activitypub_outbox_purge_days' ) ); + } }