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/cli/cli_media.php b/inc/cli/cli_media.php index 8aa31e12..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 ); + 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 cd66df44..bd80ca4e 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -1384,8 +1384,7 @@ public function generate_image_meta( $meta, $attachment_id ) { $offload_manager = Optimole::offload(); $offload_usage = $offload_manager->getUsage(); - $current_run = self::get_process_meta(); - $remaining = isset( $current_run['remaining'] ) ? absint( $current_run['remaining'] ) : 0; + $remaining = self::get_pending_offload_count(); if ( $remaining + $offload_usage->getCurrent() >= $offload_usage->getLimit() ) { if ( OPTML_DEBUG_MEDIA ) { @@ -1393,6 +1392,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.' ); @@ -1812,18 +1812,76 @@ 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'. + * @param bool $foreground Whether the transfer runs in the foreground (WP-CLI), without the + * offloading or rollback status a dashboard transfer has. The progress + * then holds the time of the run's last activity. * * @return void */ - public static function record_process_meta( $count ) { + 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 ? time() : 0; update_option( 'optml_process_meta', $meta ); } + /** + * 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'] = 0; + 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. + * + * @return int + */ + private static function get_pending_offload_count(): int { + $meta = get_option( 'optml_process_meta', [] ); + + if ( ! isset( $meta['remaining'] ) ) { + return 0; + } + + // 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 last activity: the + // mark is cleared when the command ends, and goes stale when the command was interrupted. + $running = self::is_foreground_process_alive( $meta ) || self::$instance->settings->get( 'offloading_status' ) !== 'disabled'; + + return $running ? absint( $meta['remaining'] ) : 0; + } /** * Update the process meta count. * @@ -1837,6 +1895,11 @@ 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 mark that went stale + // belongs to an interrupted run: a later upload must not bring it back. + if ( ! empty( $meta['foreground'] ) ) { + $meta['foreground'] = self::is_foreground_process_alive( $meta ) ? time() : 0; + } update_option( 'optml_process_meta', $meta ); } @@ -1898,7 +1961,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 529b5373..21e791ff 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -739,4 +739,155 @@ 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 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, 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, ?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' ); + 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(), + ]; + $offload = $progress + [ 'action' => 'offload_images' ]; + + 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' => 0 ], 'enabled', true ], + 'dashboard offload that finished or stopped at the limit' => [ $offload + [ 'foreground' => 0 ], 'disabled', false ], + '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, '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 ], + ]; + } + + 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->assertGreaterThanOrEqual( time() - 5, get_option( 'optml_process_meta' )['foreground'] ); + + Optml_Media_Offload::end_foreground_process_meta(); + + $meta = get_option( 'optml_process_meta' ); + $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'] ); + } + + /** + * 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' ); + + Optml_Media_Offload::decrement_process_meta_remaining(); + + $this->assertSame( 0, get_option( 'optml_process_meta' )['foreground'] ); + } + + /** + * @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 ); + + $meta = get_option( 'optml_process_meta' ); + $this->assertSame( $action, $meta['action'] ); + $this->assertSame( 0, $meta['foreground'] ); + } + + /** + * @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 new file mode 100644 index 00000000..ad1c764b --- /dev/null +++ b/tests/test-offload-limit-sync.php @@ -0,0 +1,129 @@ +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(): void { + $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(): void { + $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(): void { + $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' ) ); + } +}