From a9fcc18fa92fbeb980b271773fe626977042e970 Mon Sep 17 00:00:00 2001 From: selul Date: Fri, 9 Oct 2026 19:47:44 +0300 Subject: [PATCH 1/7] fix: clear a stale offload limit warning once the account is below the limit The "offload limit reached" flag was set by the upload check and cleared only when a bulk offload started, so the warning stayed after the account usage dropped (for example after duplicate sites were removed). It also silently stopped new uploads from offloading. - The daily sync stores the account's offload limit and clears the flag when the account is below it again. - The upload check stores the account limit with the flag, so the warning no longer shows the 50,000 default. - The upload check counts the images still to upload only while a bulk offload runs. The progress of a finished offload or of a rollback (which records how many images it moves back) no longer counts toward the limit. Co-Authored-By: Claude Opus 5.5 --- inc/admin.php | 10 +++ inc/media_offload.php | 8 +- tests/test-media.php | 37 +++++++++ tests/test-offload-limit-sync.php | 129 ++++++++++++++++++++++++++++++ 4 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 tests/test-offload-limit-sync.php diff --git a/inc/admin.php b/inc/admin.php index 746bdabf..72fb017f 100755 --- a/inc/admin.php +++ b/inc/admin.php @@ -1203,6 +1203,16 @@ public function daily_sync() { $this->settings->update_frontend_banner_from_remote( $data['extra_visits'] ); } + // The account usage decides the offload limit: a limit warning from an earlier upload is cleared + // once the account is below the limit again, for example after images or sites were removed. + if ( isset( $data['offload_limit'], $data['offloaded_images'] ) ) { + $this->settings->update( 'offload_limit', (int) $data['offload_limit'] ); + + if ( (int) $data['offloaded_images'] < (int) $data['offload_limit'] ) { + $this->settings->update( 'offload_limit_reached', 'disabled' ); + } + } + // Here the account got deactivated, in this case we check if the user is using offloaded images and we roll them back. $should_revert_offloading = isset( $data['status'] ) && $data['status'] === 'inactive'; diff --git a/inc/media_offload.php b/inc/media_offload.php index cd66df44..c7d79983 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -1384,8 +1384,13 @@ public function generate_image_meta( $meta, $attachment_id ) { $offload_manager = Optimole::offload(); $offload_usage = $offload_manager->getUsage(); + // Images still to upload in a running bulk offload count toward the limit. The progress of a + // finished offload or of a rollback does not. $current_run = self::get_process_meta(); - $remaining = isset( $current_run['remaining'] ) ? absint( $current_run['remaining'] ) : 0; + $remaining = 0; + if ( self::$instance->settings->get( 'offloading_status' ) !== 'disabled' && isset( $current_run['remaining'] ) ) { + $remaining = absint( $current_run['remaining'] ); + } if ( $remaining + $offload_usage->getCurrent() >= $offload_usage->getLimit() ) { if ( OPTML_DEBUG_MEDIA ) { @@ -1393,6 +1398,7 @@ public function generate_image_meta( $meta, $attachment_id ) { do_action( 'optml_log', $offload_usage ); } + self::$instance->settings->update( 'offload_limit', $offload_usage->getLimit() ); self::$instance->settings->update( 'offload_limit_reached', 'enabled' ); self::$instance->logger->add_log( Optml_Logger::LOG_TYPE_OFFLOAD, 'Offload stopped: offloading images would exceed limit.' ); diff --git a/tests/test-media.php b/tests/test-media.php index 529b5373..931bcf59 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -345,6 +345,43 @@ public function test_image_sync() { $this->assertStringContainsString( '/w:300/h:200/q:mauto/process:' . self::$sample_attachement .'/id:579c7f7707ce87caa65fdf50c238a117', $image_medium_size[0] ); } + /** + * The account mock reports 50 of 5000 images offloaded. A rollback records how many images it + * moves back as the "remaining" progress; once it is over, that number must not count toward + * the limit of the next upload. + */ + public function test_stale_transfer_progress_does_not_trip_the_offload_limit() { + $settings = new Optml_Settings(); + $settings->update( 'offloading_status', 'disabled' ); + $settings->update( 'offload_limit_reached', 'disabled' ); + Optml_Media_Offload::record_process_meta( 4990 ); + + $attachment_id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/' . self::$files[0] . '.jpg' ); + $meta = wp_get_attachment_metadata( $attachment_id ); + + $this->assertTrue( Optml_Media_Offload::is_uploaded_image( $meta['file'] ) ); + $this->assertFalse( ( new Optml_Settings() )->is_offload_limit_reached() ); + } + + /** + * During a bulk offload the images still to upload do count; when they would exceed the limit, + * the limit of the account is stored with the warning (not the 50,000 default). + */ + public function test_offload_limit_check_stores_the_account_limit() { + $settings = new Optml_Settings(); + $settings->update( 'offloading_status', 'enabled' ); + $settings->update( 'offload_limit_reached', 'disabled' ); + Optml_Media_Offload::record_process_meta( 4990 ); + + $attachment_id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/' . self::$files[0] . '.jpg' ); + $meta = wp_get_attachment_metadata( $attachment_id ); + $settings = new Optml_Settings(); + + $this->assertFalse( Optml_Media_Offload::is_uploaded_image( $meta['file'] ) ); + $this->assertTrue( $settings->is_offload_limit_reached() ); + $this->assertSame( 5000, (int) $settings->get( 'offload_limit' ) ); + } + public function test_image_rollback() { Optml_Media_Offload::instance()->rollback_images( 100 ); diff --git a/tests/test-offload-limit-sync.php b/tests/test-offload-limit-sync.php new file mode 100644 index 00000000..dabdcb04 --- /dev/null +++ b/tests/test-offload-limit-sync.php @@ -0,0 +1,129 @@ + + */ + private $account = []; + + /** + * Set up a connected site with a limit warning from an earlier upload. + */ + public function setUp(): void { + parent::setUp(); + + $settings = new Optml_Settings(); + $settings->update( 'api_key', 'test-key' ); + $settings->update( + 'service_data', + [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com' ], + ] + ); + $settings->update( 'offload_limit_reached', 'enabled' ); + $settings->update( 'offload_limit', 50000 ); + + add_filter( 'pre_http_request', [ $this, 'mock_account_details' ], 10, 3 ); + } + + /** + * Clean up after each test. + */ + public function tearDown(): void { + remove_filter( 'pre_http_request', [ $this, 'mock_account_details' ], 10 ); + parent::tearDown(); + } + + /** + * Answer the account details request with $this->account. + * + * @param false|array|WP_Error $preempt Whether to preempt. + * @param array $args Request args. + * @param string $url Request URL. + * @return false|array|WP_Error + */ + public function mock_account_details( $preempt, $args, $url ) { + if ( strpos( $url, 'optml/v2/account/details' ) === false ) { + return $preempt; + } + + return [ + 'headers' => [], + 'body' => wp_json_encode( + [ + 'code' => 200, + 'data' => array_merge( + [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com' ], + 'status' => 'active', + ], + $this->account + ), + ] + ), + 'response' => [ + 'code' => 200, + 'message' => 'OK', + ], + ]; + } + + /** + * Usage below the limit, for example after duplicate sites were removed, clears the warning. + */ + public function test_daily_sync_clears_the_warning_when_usage_is_below_the_limit() { + $this->account = [ + 'offloaded_images' => 1181, + 'offload_limit' => 20000, + ]; + + Optml_Main::instance()->admin->daily_sync(); + + $settings = new Optml_Settings(); + $this->assertFalse( $settings->is_offload_limit_reached() ); + $this->assertSame( 20000, (int) $settings->get( 'offload_limit' ) ); + } + + /** + * An account still at its limit keeps the warning. + */ + public function test_daily_sync_keeps_the_warning_at_the_limit() { + $this->account = [ + 'offloaded_images' => 20000, + 'offload_limit' => 20000, + ]; + + Optml_Main::instance()->admin->daily_sync(); + + $settings = new Optml_Settings(); + $this->assertTrue( $settings->is_offload_limit_reached() ); + $this->assertSame( 20000, (int) $settings->get( 'offload_limit' ) ); + } + + /** + * Without usage details in the response, nothing changes. + */ + public function test_daily_sync_without_usage_details_keeps_the_warning() { + $this->account = []; + + Optml_Main::instance()->admin->daily_sync(); + + $settings = new Optml_Settings(); + $this->assertTrue( $settings->is_offload_limit_reached() ); + $this->assertSame( 50000, (int) $settings->get( 'offload_limit' ) ); + } +} From 3d41c762c017b06d1b435fd7ef0aac0944302511 Mon Sep 17 00:00:00 2001 From: selul Date: Sun, 11 Oct 2026 17:11:40 +0300 Subject: [PATCH 2/7] fix: count the pending images of a WP-CLI offload toward the offload limit The upload check counted the "remaining" images only while the dashboard offload status was on, and `wp optimole media move_to_optimole` never sets that status, so a CLI offload no longer stopped before the limit. record_process_meta() now stores which transfer the progress belongs to, and the check counts it for offloads only, from the dashboard or WP-CLI. Progress recorded before this change has no transfer: it counts only while a dashboard offload runs, so stale rollback progress on existing sites stops tripping the limit. Also types the new tests, and moves the media tests away from the place where two other open PRs add theirs. Co-Authored-By: Claude Opus 5.5 --- inc/cli/cli_media.php | 2 +- inc/media_offload.php | 36 +++++++--- tests/test-media.php | 106 +++++++++++++++++++----------- tests/test-offload-limit-sync.php | 12 ++-- 4 files changed, 101 insertions(+), 55 deletions(-) diff --git a/inc/cli/cli_media.php b/inc/cli/cli_media.php index 8aa31e12..380334d7 100644 --- a/inc/cli/cli_media.php +++ b/inc/cli/cli_media.php @@ -57,7 +57,7 @@ private function update_images_template( $action ) { $number_of_images_for = 'rollback_images'; } $number_of_images = Optml_Media_Offload::number_of_images_and_pages( $number_of_images_for ); - Optml_Media_Offload::record_process_meta( $number_of_images ); + Optml_Media_Offload::record_process_meta( $number_of_images, $number_of_images_for ); $batch = 5; if ( $number_of_images === 0 ) { WP_CLI::success( $strings[ $action ]['success'] ); diff --git a/inc/media_offload.php b/inc/media_offload.php index c7d79983..463ae750 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -1384,13 +1384,7 @@ public function generate_image_meta( $meta, $attachment_id ) { $offload_manager = Optimole::offload(); $offload_usage = $offload_manager->getUsage(); - // Images still to upload in a running bulk offload count toward the limit. The progress of a - // finished offload or of a rollback does not. - $current_run = self::get_process_meta(); - $remaining = 0; - if ( self::$instance->settings->get( 'offloading_status' ) !== 'disabled' && isset( $current_run['remaining'] ) ) { - $remaining = absint( $current_run['remaining'] ); - } + $remaining = self::get_pending_offload_count(); if ( $remaining + $offload_usage->getCurrent() >= $offload_usage->getLimit() ) { if ( OPTML_DEBUG_MEDIA ) { @@ -1818,18 +1812,40 @@ function ( $image_id ) { /** * Record process meta, * - * @param int $count The number of images to process. + * @param int $count The number of images to process. + * @param string $action The transfer the images belong to, 'offload_images' or 'rollback_images'. * * @return void */ - public static function record_process_meta( $count ) { + public static function record_process_meta( $count, $action = 'offload_images' ) { $meta = get_option( 'optml_process_meta', [] ); $meta['count'] = $count; $meta['remaining'] = $count; $meta['start_time'] = time(); + $meta['action'] = $action; update_option( 'optml_process_meta', $meta ); } + /** + * Images a bulk offload still has to upload. They count toward the offload limit; the images a + * rollback still has to move back do not. + * + * @return int + */ + private static function get_pending_offload_count() { + $meta = get_option( 'optml_process_meta', [] ); + + if ( ! isset( $meta['remaining'] ) ) { + return 0; + } + + // Progress recorded before the transfer was stored: it counts while a dashboard offload runs. + $is_offload = isset( $meta['action'] ) + ? 'offload_images' === $meta['action'] + : self::$instance->settings->get( 'offloading_status' ) !== 'disabled'; + + return $is_offload ? absint( $meta['remaining'] ) : 0; + } /** * Update the process meta count. * @@ -1904,7 +1920,7 @@ public static function move_images( $action, $refresh ) { self::$instance->settings->update( 'rollback_images' === $action ? 'offloading_status' : 'rollback_status', 'disabled' ); } self::$instance->settings->update( 'offload_limit_reached', 'disabled' ); - self::record_process_meta( $count ); + self::record_process_meta( $count, $action ); self::$instance->settings->update( $option, $in_progress ? 'enabled' : 'disabled' ); self::$instance->logger->add_log( $type, Optml_Logger::LOG_SEPARATOR ); diff --git a/tests/test-media.php b/tests/test-media.php index 931bcf59..1bf44a1e 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -345,43 +345,6 @@ public function test_image_sync() { $this->assertStringContainsString( '/w:300/h:200/q:mauto/process:' . self::$sample_attachement .'/id:579c7f7707ce87caa65fdf50c238a117', $image_medium_size[0] ); } - /** - * The account mock reports 50 of 5000 images offloaded. A rollback records how many images it - * moves back as the "remaining" progress; once it is over, that number must not count toward - * the limit of the next upload. - */ - public function test_stale_transfer_progress_does_not_trip_the_offload_limit() { - $settings = new Optml_Settings(); - $settings->update( 'offloading_status', 'disabled' ); - $settings->update( 'offload_limit_reached', 'disabled' ); - Optml_Media_Offload::record_process_meta( 4990 ); - - $attachment_id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/' . self::$files[0] . '.jpg' ); - $meta = wp_get_attachment_metadata( $attachment_id ); - - $this->assertTrue( Optml_Media_Offload::is_uploaded_image( $meta['file'] ) ); - $this->assertFalse( ( new Optml_Settings() )->is_offload_limit_reached() ); - } - - /** - * During a bulk offload the images still to upload do count; when they would exceed the limit, - * the limit of the account is stored with the warning (not the 50,000 default). - */ - public function test_offload_limit_check_stores_the_account_limit() { - $settings = new Optml_Settings(); - $settings->update( 'offloading_status', 'enabled' ); - $settings->update( 'offload_limit_reached', 'disabled' ); - Optml_Media_Offload::record_process_meta( 4990 ); - - $attachment_id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/' . self::$files[0] . '.jpg' ); - $meta = wp_get_attachment_metadata( $attachment_id ); - $settings = new Optml_Settings(); - - $this->assertFalse( Optml_Media_Offload::is_uploaded_image( $meta['file'] ) ); - $this->assertTrue( $settings->is_offload_limit_reached() ); - $this->assertSame( 5000, (int) $settings->get( 'offload_limit' ) ); - } - public function test_image_rollback() { Optml_Media_Offload::instance()->rollback_images( 100 ); @@ -776,4 +739,71 @@ public function test_move_images_does_not_schedule_duplicate_processing_chain() delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); } -} \ No newline at end of file + + /** + * The account mock reports 50 of 5000 images offloaded, and 4990 images are recorded as still to + * be moved. They count toward the limit of an upload only when they belong to an offload: a + * rollback records the images it moves back in the same place. + * + * @dataProvider pending_transfer_provider + * + * @param array{count: int, remaining: int, start_time: int, action?: string} $process_meta The recorded transfer progress. + * @param string $offloading_status The dashboard offload status. + * @param bool $limit_reached Whether the upload trips the limit. + */ + public function test_only_a_pending_offload_counts_toward_the_offload_limit( array $process_meta, string $offloading_status, bool $limit_reached ): void { + $settings = new Optml_Settings(); + $settings->update( 'offloading_status', $offloading_status ); + $settings->update( 'offload_limit_reached', 'disabled' ); + update_option( 'optml_process_meta', $process_meta ); + + $attachment_id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/' . self::$files[0] . '.jpg' ); + $meta = wp_get_attachment_metadata( $attachment_id ); + $settings = new Optml_Settings(); + + $this->assertSame( ! $limit_reached, Optml_Media_Offload::is_uploaded_image( $meta['file'] ) ); + $this->assertSame( $limit_reached, $settings->is_offload_limit_reached() ); + + if ( $limit_reached ) { + // The limit of the account is stored with the warning, not the 50,000 default. + $this->assertSame( 5000, (int) $settings->get( 'offload_limit' ) ); + } + } + + /** + * @return array + */ + public function pending_transfer_provider(): array { + $progress = [ + 'count' => 4990, + 'remaining' => 4990, + 'start_time' => time(), + ]; + + return [ + 'rollback' => [ $progress + [ 'action' => 'rollback_images' ], 'disabled', false ], + 'offload, also from WP-CLI (no dashboard status)' => [ $progress + [ 'action' => 'offload_images' ], 'disabled', true ], + 'recorded before the transfer was stored, idle' => [ $progress, 'disabled', false ], + 'recorded before the transfer was stored, offload running' => [ $progress, 'enabled', true ], + ]; + } + + /** + * @dataProvider transfer_action_provider + */ + public function test_a_bulk_transfer_records_which_transfer_its_progress_belongs_to( string $action ): void { + Optml_Media_Offload::move_images( $action, false ); + + $this->assertSame( $action, get_option( 'optml_process_meta' )['action'] ); + } + + /** + * @return array + */ + public function transfer_action_provider(): array { + return [ + 'offload' => [ 'offload_images' ], + 'rollback' => [ 'rollback_images' ], + ]; + } +} diff --git a/tests/test-offload-limit-sync.php b/tests/test-offload-limit-sync.php index dabdcb04..ad1c764b 100644 --- a/tests/test-offload-limit-sync.php +++ b/tests/test-offload-limit-sync.php @@ -10,11 +10,11 @@ */ class Test_Offload_Limit_Sync extends WP_UnitTestCase { /** - * Account details the mocked API returns. + * Usage details the mocked account API returns. * - * @var array + * @var array{offloaded_images?: int, offload_limit?: int} */ - private $account = []; + private array $account = []; /** * Set up a connected site with a limit warning from an earlier upload. @@ -85,7 +85,7 @@ public function mock_account_details( $preempt, $args, $url ) { /** * Usage below the limit, for example after duplicate sites were removed, clears the warning. */ - public function test_daily_sync_clears_the_warning_when_usage_is_below_the_limit() { + public function test_daily_sync_clears_the_warning_when_usage_is_below_the_limit(): void { $this->account = [ 'offloaded_images' => 1181, 'offload_limit' => 20000, @@ -101,7 +101,7 @@ public function test_daily_sync_clears_the_warning_when_usage_is_below_the_limit /** * An account still at its limit keeps the warning. */ - public function test_daily_sync_keeps_the_warning_at_the_limit() { + public function test_daily_sync_keeps_the_warning_at_the_limit(): void { $this->account = [ 'offloaded_images' => 20000, 'offload_limit' => 20000, @@ -117,7 +117,7 @@ public function test_daily_sync_keeps_the_warning_at_the_limit() { /** * Without usage details in the response, nothing changes. */ - public function test_daily_sync_without_usage_details_keeps_the_warning() { + public function test_daily_sync_without_usage_details_keeps_the_warning(): void { $this->account = []; Optml_Main::instance()->admin->daily_sync(); From e2b4f490d0fdaeb908482d5c772276e847ef0f67 Mon Sep 17 00:00:00 2001 From: selul Date: Sun, 11 Oct 2026 18:13:26 +0300 Subject: [PATCH 3/7] fix: count pending offload images only while that offload runs "remaining" stays in the progress after an offload finished, was cancelled or stopped at the limit, so it kept counting toward the limit of later uploads: after the daily sync cleared the warning, the next upload set it again. It now counts only while the offload is going: a dashboard offload while its status is on, and one from WP-CLI while the command runs. The command marks its progress as a foreground run and clears the mark when it ends (shutdown function, so also on an error exit). A rollback never counts. Also adds native types to record_process_meta() and the new helper. Co-Authored-By: Claude Opus 5.5 --- inc/cli/cli_media.php | 4 +++- inc/media_offload.php | 44 ++++++++++++++++++++++++++++++++----------- tests/test-media.php | 40 ++++++++++++++++++++++++++++----------- 3 files changed, 65 insertions(+), 23 deletions(-) diff --git a/inc/cli/cli_media.php b/inc/cli/cli_media.php index 380334d7..5f8d1012 100644 --- a/inc/cli/cli_media.php +++ b/inc/cli/cli_media.php @@ -57,7 +57,9 @@ private function update_images_template( $action ) { $number_of_images_for = 'rollback_images'; } $number_of_images = Optml_Media_Offload::number_of_images_and_pages( $number_of_images_for ); - Optml_Media_Offload::record_process_meta( $number_of_images, $number_of_images_for ); + Optml_Media_Offload::record_process_meta( $number_of_images, $number_of_images_for, true ); + // The run is over when this command ends, however it exits. + register_shutdown_function( [ 'Optml_Media_Offload', 'end_foreground_process_meta' ] ); $batch = 5; if ( $number_of_images === 0 ) { WP_CLI::success( $strings[ $action ]['success'] ); diff --git a/inc/media_offload.php b/inc/media_offload.php index 463ae750..11b75954 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -1812,39 +1812,61 @@ function ( $image_id ) { /** * Record process meta, * - * @param int $count The number of images to process. - * @param string $action The transfer the images belong to, 'offload_images' or 'rollback_images'. + * @param int $count The number of images to process. + * @param string $action The transfer the images belong to, 'offload_images' or 'rollback_images'. + * @param bool $foreground Whether the transfer runs in the foreground (WP-CLI), without the + * offloading or rollback status a dashboard transfer has. * * @return void */ - public static function record_process_meta( $count, $action = 'offload_images' ) { + public static function record_process_meta( int $count, $action = 'offload_images', bool $foreground = false ): void { $meta = get_option( 'optml_process_meta', [] ); $meta['count'] = $count; $meta['remaining'] = $count; $meta['start_time'] = time(); $meta['action'] = $action; + $meta['foreground'] = $foreground; update_option( 'optml_process_meta', $meta ); } /** - * Images a bulk offload still has to upload. They count toward the offload limit; the images a - * rollback still has to move back do not. + * Mark the transfer that runs in the foreground as over, see {@see self::record_process_meta()}. + * + * @return void + */ + public static function end_foreground_process_meta(): void { + $meta = get_option( 'optml_process_meta', [] ); + + if ( empty( $meta['foreground'] ) ) { + return; + } + + $meta['foreground'] = false; + update_option( 'optml_process_meta', $meta ); + } + + /** + * Images a bulk offload that is still going has to upload. They count toward the offload limit. * * @return int */ - private static function get_pending_offload_count() { + private static function get_pending_offload_count(): int { $meta = get_option( 'optml_process_meta', [] ); if ( ! isset( $meta['remaining'] ) ) { return 0; } - // Progress recorded before the transfer was stored: it counts while a dashboard offload runs. - $is_offload = isset( $meta['action'] ) - ? 'offload_images' === $meta['action'] - : self::$instance->settings->get( 'offloading_status' ) !== 'disabled'; + // A rollback records the images it still has to move back in the same place. + if ( isset( $meta['action'] ) && 'offload_images' !== $meta['action'] ) { + return 0; + } + + // "remaining" stays after an offload finished, was cancelled or stopped at the limit. A + // dashboard offload runs while its status is on; one from WP-CLI marks its own run. + $running = ! empty( $meta['foreground'] ) || self::$instance->settings->get( 'offloading_status' ) !== 'disabled'; - return $is_offload ? absint( $meta['remaining'] ) : 0; + return $running ? absint( $meta['remaining'] ) : 0; } /** * Update the process meta count. diff --git a/tests/test-media.php b/tests/test-media.php index 1bf44a1e..6650a834 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -742,16 +742,16 @@ public function test_move_images_does_not_schedule_duplicate_processing_chain() /** * The account mock reports 50 of 5000 images offloaded, and 4990 images are recorded as still to - * be moved. They count toward the limit of an upload only when they belong to an offload: a - * rollback records the images it moves back in the same place. + * be moved. They count toward the limit of an upload only while an offload is moving them: not + * for a rollback, which records its progress in the same place, and not once the offload is over. * * @dataProvider pending_transfer_provider * - * @param array{count: int, remaining: int, start_time: int, action?: string} $process_meta The recorded transfer progress. - * @param string $offloading_status The dashboard offload status. - * @param bool $limit_reached Whether the upload trips the limit. + * @param array{count: int, remaining: int, start_time: int, action?: string, foreground?: bool} $process_meta The recorded transfer progress. + * @param string $offloading_status The dashboard offload status. + * @param bool $limit_reached Whether the upload trips the limit. */ - public function test_only_a_pending_offload_counts_toward_the_offload_limit( array $process_meta, string $offloading_status, bool $limit_reached ): void { + public function test_only_a_running_offload_counts_toward_the_offload_limit( array $process_meta, string $offloading_status, bool $limit_reached ): void { $settings = new Optml_Settings(); $settings->update( 'offloading_status', $offloading_status ); $settings->update( 'offload_limit_reached', 'disabled' ); @@ -771,7 +771,7 @@ public function test_only_a_pending_offload_counts_toward_the_offload_limit( arr } /** - * @return array + * @return array */ public function pending_transfer_provider(): array { $progress = [ @@ -779,22 +779,40 @@ public function pending_transfer_provider(): array { 'remaining' => 4990, 'start_time' => time(), ]; + $offload = $progress + [ 'action' => 'offload_images' ]; return [ - 'rollback' => [ $progress + [ 'action' => 'rollback_images' ], 'disabled', false ], - 'offload, also from WP-CLI (no dashboard status)' => [ $progress + [ 'action' => 'offload_images' ], 'disabled', true ], - 'recorded before the transfer was stored, idle' => [ $progress, 'disabled', false ], + 'rollback' => [ $progress + [ 'action' => 'rollback_images' ], 'disabled', false ], + 'rollback, with a stale offload status' => [ $progress + [ 'action' => 'rollback_images' ], 'enabled', false ], + 'dashboard offload that is running' => [ $offload + [ 'foreground' => false ], 'enabled', true ], + 'dashboard offload that finished or stopped at the limit' => [ $offload + [ 'foreground' => false ], 'disabled', false ], + 'WP-CLI offload that is running' => [ $offload + [ 'foreground' => true ], 'disabled', true ], + 'WP-CLI offload that ended' => [ $offload + [ 'foreground' => false ], 'disabled', false ], + 'recorded before the transfer was stored, idle' => [ $progress, 'disabled', false ], 'recorded before the transfer was stored, offload running' => [ $progress, 'enabled', true ], ]; } + public function test_a_wp_cli_run_marks_its_progress_until_it_ends(): void { + Optml_Media_Offload::record_process_meta( 12, 'offload_images', true ); + $this->assertTrue( get_option( 'optml_process_meta' )['foreground'] ); + + Optml_Media_Offload::end_foreground_process_meta(); + + $meta = get_option( 'optml_process_meta' ); + $this->assertFalse( $meta['foreground'] ); + $this->assertSame( 12, $meta['remaining'] ); + } + /** * @dataProvider transfer_action_provider */ public function test_a_bulk_transfer_records_which_transfer_its_progress_belongs_to( string $action ): void { Optml_Media_Offload::move_images( $action, false ); - $this->assertSame( $action, get_option( 'optml_process_meta' )['action'] ); + $meta = get_option( 'optml_process_meta' ); + $this->assertSame( $action, $meta['action'] ); + $this->assertFalse( $meta['foreground'] ); } /** From c16bb44c050a6110d518e0ad6fab1cef91732e49 Mon Sep 17 00:00:00 2001 From: selul Date: Sun, 11 Oct 2026 19:01:09 +0300 Subject: [PATCH 4/7] fix: let the mark of a WP-CLI offload go stale when the command is interrupted The shutdown function that clears the mark does not run on Ctrl+C or SIGTERM, so an interrupted command kept counting its pending images toward the offload limit. The mark is now the time of the run's last activity: each offloaded image refreshes it, and it counts only while it is younger than the lifetime of a transfer step (10 minutes). Co-Authored-By: Claude Opus 5.5 --- inc/media_offload.php | 17 ++++++++++----- tests/test-media.php | 50 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/inc/media_offload.php b/inc/media_offload.php index 11b75954..4974f54a 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -1815,7 +1815,8 @@ function ( $image_id ) { * @param int $count The number of images to process. * @param string $action The transfer the images belong to, 'offload_images' or 'rollback_images'. * @param bool $foreground Whether the transfer runs in the foreground (WP-CLI), without the - * offloading or rollback status a dashboard transfer has. + * offloading or rollback status a dashboard transfer has. The progress + * then holds the time of the run's last activity. * * @return void */ @@ -1825,7 +1826,7 @@ public static function record_process_meta( int $count, $action = 'offload_image $meta['remaining'] = $count; $meta['start_time'] = time(); $meta['action'] = $action; - $meta['foreground'] = $foreground; + $meta['foreground'] = $foreground ? time() : 0; update_option( 'optml_process_meta', $meta ); } @@ -1841,7 +1842,7 @@ public static function end_foreground_process_meta(): void { return; } - $meta['foreground'] = false; + $meta['foreground'] = 0; update_option( 'optml_process_meta', $meta ); } @@ -1863,8 +1864,10 @@ private static function get_pending_offload_count(): int { } // "remaining" stays after an offload finished, was cancelled or stopped at the limit. A - // dashboard offload runs while its status is on; one from WP-CLI marks its own run. - $running = ! empty( $meta['foreground'] ) || self::$instance->settings->get( 'offloading_status' ) !== 'disabled'; + // dashboard offload runs while its status is on. One from WP-CLI marks its last activity: the + // mark is cleared when the command ends, and goes stale when the command was interrupted. + $foreground_running = ! empty( $meta['foreground'] ) && ( time() - (int) $meta['foreground'] ) < self::TRANSFER_LOCK_TTL; + $running = $foreground_running || self::$instance->settings->get( 'offloading_status' ) !== 'disabled'; return $running ? absint( $meta['remaining'] ) : 0; } @@ -1881,6 +1884,10 @@ public static function decrement_process_meta_remaining() { } $meta['remaining'] = $meta['remaining'] - 1; + // A run in the foreground is alive as long as it keeps moving images. + if ( ! empty( $meta['foreground'] ) ) { + $meta['foreground'] = time(); + } update_option( 'optml_process_meta', $meta ); } diff --git a/tests/test-media.php b/tests/test-media.php index 6650a834..f5d81e48 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -747,7 +747,7 @@ public function test_move_images_does_not_schedule_duplicate_processing_chain() * * @dataProvider pending_transfer_provider * - * @param array{count: int, remaining: int, start_time: int, action?: string, foreground?: bool} $process_meta The recorded transfer progress. + * @param array{count: int, remaining: int, start_time: int, action?: string, foreground?: int} $process_meta The recorded transfer progress. * @param string $offloading_status The dashboard offload status. * @param bool $limit_reached Whether the upload trips the limit. */ @@ -771,7 +771,7 @@ public function test_only_a_running_offload_counts_toward_the_offload_limit( arr } /** - * @return array + * @return array */ public function pending_transfer_provider(): array { $progress = [ @@ -784,10 +784,11 @@ public function pending_transfer_provider(): array { return [ 'rollback' => [ $progress + [ 'action' => 'rollback_images' ], 'disabled', false ], 'rollback, with a stale offload status' => [ $progress + [ 'action' => 'rollback_images' ], 'enabled', false ], - 'dashboard offload that is running' => [ $offload + [ 'foreground' => false ], 'enabled', true ], - 'dashboard offload that finished or stopped at the limit' => [ $offload + [ 'foreground' => false ], 'disabled', false ], - 'WP-CLI offload that is running' => [ $offload + [ 'foreground' => true ], 'disabled', true ], - 'WP-CLI offload that ended' => [ $offload + [ 'foreground' => false ], 'disabled', false ], + 'dashboard offload that is running' => [ $offload + [ 'foreground' => 0 ], 'enabled', true ], + 'dashboard offload that finished or stopped at the limit' => [ $offload + [ 'foreground' => 0 ], 'disabled', false ], + 'WP-CLI offload that is running' => [ $offload + [ 'foreground' => time() ], 'disabled', true ], + 'WP-CLI offload that ended' => [ $offload + [ 'foreground' => 0 ], 'disabled', false ], + 'WP-CLI offload interrupted an hour ago (Ctrl+C)' => [ $offload + [ 'foreground' => time() - HOUR_IN_SECONDS ], 'disabled', false ], 'recorded before the transfer was stored, idle' => [ $progress, 'disabled', false ], 'recorded before the transfer was stored, offload running' => [ $progress, 'enabled', true ], ]; @@ -795,15 +796,46 @@ public function pending_transfer_provider(): array { public function test_a_wp_cli_run_marks_its_progress_until_it_ends(): void { Optml_Media_Offload::record_process_meta( 12, 'offload_images', true ); - $this->assertTrue( get_option( 'optml_process_meta' )['foreground'] ); + $this->assertGreaterThanOrEqual( time() - 5, get_option( 'optml_process_meta' )['foreground'] ); Optml_Media_Offload::end_foreground_process_meta(); $meta = get_option( 'optml_process_meta' ); - $this->assertFalse( $meta['foreground'] ); + $this->assertSame( 0, $meta['foreground'] ); $this->assertSame( 12, $meta['remaining'] ); } + /** + * The mark of a WP-CLI run is the time of its last activity: each offloaded image refreshes it, + * so it goes stale by itself when the command is interrupted. + */ + public function test_each_offloaded_image_refreshes_the_mark_of_a_wp_cli_run(): void { + update_option( + 'optml_process_meta', + [ + 'count' => 12, + 'remaining' => 12, + 'start_time' => time() - 500, + 'action' => 'offload_images', + 'foreground' => time() - 500, + ] + ); + + Optml_Media_Offload::decrement_process_meta_remaining(); + + $meta = get_option( 'optml_process_meta' ); + $this->assertSame( 11, $meta['remaining'] ); + $this->assertGreaterThanOrEqual( time() - 5, $meta['foreground'] ); + } + + public function test_offloading_an_image_does_not_mark_a_dashboard_offload_as_a_wp_cli_run(): void { + Optml_Media_Offload::record_process_meta( 12, 'offload_images' ); + + Optml_Media_Offload::decrement_process_meta_remaining(); + + $this->assertSame( 0, get_option( 'optml_process_meta' )['foreground'] ); + } + /** * @dataProvider transfer_action_provider */ @@ -812,7 +844,7 @@ public function test_a_bulk_transfer_records_which_transfer_its_progress_belongs $meta = get_option( 'optml_process_meta' ); $this->assertSame( $action, $meta['action'] ); - $this->assertFalse( $meta['foreground'] ); + $this->assertSame( 0, $meta['foreground'] ); } /** From 0906a79fc1ebc1936af22ad88591a540db3fb381 Mon Sep 17 00:00:00 2001 From: selul Date: Sun, 11 Oct 2026 19:32:45 +0300 Subject: [PATCH 5/7] fix: do not let a later upload revive the mark of an interrupted WP-CLI offload The mark of a WP-CLI offload expires when the command stops refreshing it. A later offloaded image refreshed the mark even after it had expired, which made the stale progress count again. The mark is now refreshed only while it is still alive and cleared otherwise. Co-Authored-By: Claude Opus 5.5 --- inc/media_offload.php | 20 ++++++++++++++++---- tests/test-media.php | 28 ++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/inc/media_offload.php b/inc/media_offload.php index 4974f54a..bd80ca4e 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -1846,6 +1846,18 @@ public static function end_foreground_process_meta(): void { update_option( 'optml_process_meta', $meta ); } + /** + * Whether the progress belongs to a run in the foreground (WP-CLI) that moved an image within the + * lifetime of a transfer step. + * + * @param array $meta The process meta. + * + * @return bool + */ + private static function is_foreground_process_alive( array $meta ): bool { + return ! empty( $meta['foreground'] ) && ( time() - (int) $meta['foreground'] ) < self::TRANSFER_LOCK_TTL; + } + /** * Images a bulk offload that is still going has to upload. They count toward the offload limit. * @@ -1866,8 +1878,7 @@ private static function get_pending_offload_count(): int { // "remaining" stays after an offload finished, was cancelled or stopped at the limit. A // dashboard offload runs while its status is on. One from WP-CLI marks its last activity: the // mark is cleared when the command ends, and goes stale when the command was interrupted. - $foreground_running = ! empty( $meta['foreground'] ) && ( time() - (int) $meta['foreground'] ) < self::TRANSFER_LOCK_TTL; - $running = $foreground_running || self::$instance->settings->get( 'offloading_status' ) !== 'disabled'; + $running = self::is_foreground_process_alive( $meta ) || self::$instance->settings->get( 'offloading_status' ) !== 'disabled'; return $running ? absint( $meta['remaining'] ) : 0; } @@ -1884,9 +1895,10 @@ public static function decrement_process_meta_remaining() { } $meta['remaining'] = $meta['remaining'] - 1; - // A run in the foreground is alive as long as it keeps moving images. + // A run in the foreground is alive as long as it keeps moving images. A mark that went stale + // belongs to an interrupted run: a later upload must not bring it back. if ( ! empty( $meta['foreground'] ) ) { - $meta['foreground'] = time(); + $meta['foreground'] = self::is_foreground_process_alive( $meta ) ? time() : 0; } update_option( 'optml_process_meta', $meta ); } diff --git a/tests/test-media.php b/tests/test-media.php index f5d81e48..1a04c2aa 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -828,6 +828,34 @@ public function test_each_offloaded_image_refreshes_the_mark_of_a_wp_cli_run(): $this->assertGreaterThanOrEqual( time() - 5, $meta['foreground'] ); } + /** + * Every offloaded image updates the progress, also an ordinary upload long after a WP-CLI run was + * interrupted. It must not bring the stale mark of that run back. + */ + public function test_a_later_upload_does_not_revive_the_mark_of_an_interrupted_wp_cli_run(): void { + update_option( + 'optml_process_meta', + [ + 'count' => 4990, + 'remaining' => 4990, + 'start_time' => time() - HOUR_IN_SECONDS, + 'action' => 'offload_images', + 'foreground' => time() - HOUR_IN_SECONDS, + ] + ); + $settings = new Optml_Settings(); + $settings->update( 'offloading_status', 'disabled' ); + $settings->update( 'offload_limit_reached', 'disabled' ); + + // Two ordinary uploads: the first one updates the progress, the second one must still be offloaded. + self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/' . self::$files[0] . '.jpg' ); + $attachment_id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/' . self::$files[1] . '.jpg' ); + + $this->assertSame( 0, get_option( 'optml_process_meta' )['foreground'] ); + $this->assertTrue( Optml_Media_Offload::is_uploaded_image( wp_get_attachment_metadata( $attachment_id )['file'] ) ); + $this->assertFalse( ( new Optml_Settings() )->is_offload_limit_reached() ); + } + public function test_offloading_an_image_does_not_mark_a_dashboard_offload_as_a_wp_cli_run(): void { Optml_Media_Offload::record_process_meta( 12, 'offload_images' ); From 3dddf4af8de57f99f6170f967b1a4e5d6885b182 Mon Sep 17 00:00:00 2001 From: selul Date: Sun, 11 Oct 2026 20:20:04 +0300 Subject: [PATCH 6/7] test: date the WP-CLI mark when the test runs, not when the suite is built Data providers run when the suite is built. The 'running WP-CLI offload' row took time() there, so it would fail once more than ten minutes pass before the test runs. The provider now gives the age of the mark and the test dates it. Co-Authored-By: Claude Opus 5.5 --- tests/test-media.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/test-media.php b/tests/test-media.php index 1a04c2aa..f5fa79fe 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -750,8 +750,14 @@ public function test_move_images_does_not_schedule_duplicate_processing_chain() * @param array{count: int, remaining: int, start_time: int, action?: string, foreground?: int} $process_meta The recorded transfer progress. * @param string $offloading_status The dashboard offload status. * @param bool $limit_reached Whether the upload trips the limit. + * @param int|null $mark_age Seconds since the last activity of a WP-CLI run, null without one. */ - public function test_only_a_running_offload_counts_toward_the_offload_limit( array $process_meta, string $offloading_status, bool $limit_reached ): void { + public function test_only_a_running_offload_counts_toward_the_offload_limit( array $process_meta, string $offloading_status, bool $limit_reached, ?int $mark_age = null ): void { + // The providers run when the suite is built, long before this test: the mark is dated here. + if ( null !== $mark_age ) { + $process_meta['foreground'] = time() - $mark_age; + } + $settings = new Optml_Settings(); $settings->update( 'offloading_status', $offloading_status ); $settings->update( 'offload_limit_reached', 'disabled' ); @@ -786,9 +792,9 @@ public function pending_transfer_provider(): array { 'rollback, with a stale offload status' => [ $progress + [ 'action' => 'rollback_images' ], 'enabled', false ], 'dashboard offload that is running' => [ $offload + [ 'foreground' => 0 ], 'enabled', true ], 'dashboard offload that finished or stopped at the limit' => [ $offload + [ 'foreground' => 0 ], 'disabled', false ], - 'WP-CLI offload that is running' => [ $offload + [ 'foreground' => time() ], 'disabled', true ], + 'WP-CLI offload that is running' => [ $offload, 'disabled', true, 0 ], 'WP-CLI offload that ended' => [ $offload + [ 'foreground' => 0 ], 'disabled', false ], - 'WP-CLI offload interrupted an hour ago (Ctrl+C)' => [ $offload + [ 'foreground' => time() - HOUR_IN_SECONDS ], 'disabled', false ], + 'WP-CLI offload interrupted an hour ago (Ctrl+C)' => [ $offload, 'disabled', false, HOUR_IN_SECONDS ], 'recorded before the transfer was stored, idle' => [ $progress, 'disabled', false ], 'recorded before the transfer was stored, offload running' => [ $progress, 'enabled', true ], ]; From a34edda3e7007dff38e88a5a02d560e7ac90e2d6 Mon Sep 17 00:00:00 2001 From: selul Date: Sun, 11 Oct 2026 20:41:47 +0300 Subject: [PATCH 7/7] test: declare the optional fourth element of the pending transfer provider Co-Authored-By: Claude Opus 5.5 --- tests/test-media.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-media.php b/tests/test-media.php index f5fa79fe..21e791ff 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -777,7 +777,7 @@ public function test_only_a_running_offload_counts_toward_the_offload_limit( arr } /** - * @return array + * @return array */ public function pending_transfer_provider(): array { $progress = [