Release

inc/main.php

@@ -157,6 +157,8 @@ public static function register_conflicts( $conflicts_to_register = [] ) {
 	 * @return array
 	 */
 	public static function add_settings( $data ): array {
+		$data = is_array( $data ) ? $data : [];
+
 		$saved_data = ( new Optml_Settings() )->get_raw_settings();
 		unset( $saved_data['service_data'] );
 		unset( $saved_data['api_key'] );

inc/media_offload.php

@@ -56,6 +56,17 @@ class Optml_Media_Offload extends Optml_App_Replacer {
 	const POST_OFFLOADED_FLAG = 'optimole_offload_post';
 	const POST_ROLLBACK_FLAG = 'optimole_rollback_post';
 	const RETRYABLE_META_COUNTER = '_optimole_retryable_errors';
+
+	/**
+	 * Transient name for the transfer lock.
+	 */
+	const TRANSFER_LOCK_TRANSIENT = 'optml_transfer_lock';
+
+	/**
+	 * Time to live for the transfer lock, in seconds.
+	 */
+	const TRANSFER_LOCK_TTL = 600;
+
 	/**
 	 * Flag used inside wp_get_attachment url filter.
 	 *
@@ -173,7 +184,7 @@ public static function instance() {
 					add_filter( 'wp_insert_attachment_data', [ self::$instance, 'insert' ], 10, 4 );
 				}
 
-				add_action( 'optml_start_processing_images', [ self::$instance, 'start_processing_images' ], 10, 5 );
+				add_action( 'optml_start_processing_images', [ self::$instance, 'start_processing_images' ], 10, 6 );
 				add_action(
 					'optml_move_images_by_id',
 					[
@@ -200,8 +211,9 @@ public static function instance() {
 	 * @return void
 	 */
 	public function maybe_reschedule() {
+		$lock = get_transient( self::TRANSFER_LOCK_TRANSIENT );
 		// If this is in pending, we do nothing.
-		if ( self::is_scheduled( 'optml_start_processing_images' ) ) {
+		if ( false !== $lock ) {
 			return;
 		}
 		// If there is no transfer in progress, we do nothing.
@@ -1899,18 +1911,25 @@ public static function move_images( $action, $refresh ) {
 					'action' => $type,
 				];
 			}
-			$total = ceil( $count / $batch );
-			self::schedule_action(
-				time(),
-				'optml_start_processing_images',
-				[
-					$action,
-					$batch,
-					1,
-					$total,
-					$step,
-				]
-			);
+
+			// We acquire a lock to prevent multiple workers from running the same action concurrently.
+			$lock_token = self::acquire_transfer_lock( $action );
+
+			if ( false !== $lock_token ) {
+				$total = ceil( $count / $batch );
+				self::schedule_action(
+					time(),
+					'optml_start_processing_images',
+					[
+						$action,
+						$batch,
+						1,
+						$total,
+						$step,
+						$lock_token,
+					]
+				);
+			}
 		}
 
 		$response = [
@@ -1970,6 +1989,77 @@ public static function is_scheduled( $hook ) {
 		}
 	}
 
+	/**
+	 * Attempt to acquire the transfer lock for a given action.
+	 *
+	 * @param string $action The transfer action ('offload_images'|'rollback_images').
+	 *
+	 * @return string|false The lock token on success, false if another worker already holds the lock.
+	 */
+	public static function acquire_transfer_lock( $action ) {
+		$lock = get_transient( self::TRANSFER_LOCK_TRANSIENT );
+		if ( false !== $lock ) {
+			return false;
+		}
+
+		$token = wp_generate_uuid4();
+
+		set_transient(
+			self::TRANSFER_LOCK_TRANSIENT,
+			[
+				'token'  => $token,
+				'action' => $action,
+			],
+			self::TRANSFER_LOCK_TTL
+		);
+
+		return $token;
+	}
+
+	/**
+	 * Renew the transfer lock if we still own it, extending its expiration.
+	 *
+	 * @param string $token The lock token this worker was given when it started the chain.
+	 * @param string $action The transfer action currently being processed.
+	 *
+	 * @return bool True if we still own the lock and renewed it, false if ownership was lost.
+	 */
+	public static function renew_transfer_lock( $token, $action ) {
+		$lock = get_transient( self::TRANSFER_LOCK_TRANSIENT );
+
+		if ( ! is_array( $lock ) || ! isset( $lock['token'] ) || $lock['token'] !== $token ) {
+			return false;
+		}
+
+		set_transient(
+			self::TRANSFER_LOCK_TRANSIENT,
+			[
+				'token'  => $token,
+				'action' => $action,
+			],
+			self::TRANSFER_LOCK_TTL
+		);
+
+		return true;
+	}
+
+	/**
+	 * Release the transfer lock if we still own it, allowing another worker to acquire it.
+	 *
+	 * @param string $token The lock token to release.
+	 *
+	 * @return void
+	 */
+	public static function release_transfer_lock( $token ) {
+		$lock = get_transient( self::TRANSFER_LOCK_TRANSIENT );
+
+		if ( ! is_array( $lock ) || ! isset( $lock['token'] ) || $lock['token'] !== $token ) {
+			return;
+		}
+
+		delete_transient( self::TRANSFER_LOCK_TRANSIENT );
+	}
+
 	/**
 	 * Start Processing Images by IDs
 	 *
@@ -2029,14 +2119,21 @@ public function move_single_image( $action, $id ) {
 	 * @param int    $page The page of images to process.
 	 * @param int    $total The total number of pages.
 	 * @param int    $step The current step.
+	 * @param string $lock_token The transfer lock token owned by this processing chain.
 	 *
 	 * @return void
 	 */
-	public function start_processing_images( $action, $batch, $page, $total, $step ) {
+	public function start_processing_images( $action, $batch, $page, $total, $step, $lock_token = '' ) {
 		$option = 'offload_images' === $action ? 'offloading_status' : 'rollback_status';
 		$type   = 'offload_images' === $action ? 'offload' : 'rollback';
 
 		if ( self::$instance->settings->get( $option ) === 'disabled' ) {
+			self::release_transfer_lock( $lock_token );
+			return;
+		}
+
+		// If we don't own the lock anymore, stop processing.
+		if ( ! self::renew_transfer_lock( $lock_token, $action ) ) {
 			return;
 		}
 
@@ -2048,6 +2145,9 @@ public function start_processing_images( $action, $batch, $page, $total, $step )
 
 			self::$instance->settings->update( 'show_offload_finish_notice', $type );
 
+			// Transfer completed successfully: release the lock.
+			self::release_transfer_lock( $lock_token );
+
 			return;
 		}
 
@@ -2080,10 +2180,12 @@ public function start_processing_images( $action, $batch, $page, $total, $step )
 					$page,
 					$total,
 					$step,
+					$lock_token,
 				]
 			);
 		} catch ( Exception $e ) {
 			// Reschedule the cron to run again after a delay. Sometimes memory limit is exausted.
+			// This is a retryable error, so the lock is kept rather than released.
 			$delay_in_seconds = 10;
 			self::$instance->logger->add_log( $type, $e->getMessage() );
 
@@ -2096,6 +2198,7 @@ public function start_processing_images( $action, $batch, $page, $total, $step )
 					$page,
 					$total,
 					$step,
+					$lock_token,
 				]
 			);
 		}

inc/settings.php

@@ -783,7 +783,9 @@ public function reset() {
 	 * @return  array
 	 */
 	public function get_raw_settings() {
-		return get_option( $this->namespace, false );
+		$raw_settings = get_option( $this->namespace, [] );
+
+		return is_array( $raw_settings ) ? $raw_settings : [];
 	}
 
 

tests/test-logger-data.php

@@ -0,0 +1,84 @@
+<?php
+/**
+ * WordPress unit test plugin.
+ *
+ * Covers the `optimole_wp_logger_data` filter used by the SDK logger cron
+ * (`optimole_wp_log_activity`) when no settings have been stored yet.
+ *
+ * @package     Optimole-WP
+ * @subpackage  Tests
+ * @license     http://opensource.org/licenses/gpl-2.0.php GNU Public License
+ */
+class Test_Logger_Data extends WP_UnitTestCase {
+
+	public function tearDown(): void {
+		parent::tearDown();
+
+		// The rollback restored the option, refresh the static settings cache from it.
+		new Optml_Settings();
+	}
+
+	/**
+	 * Raw settings must be an array even when the option was never stored.
+	 */
+	public function test_get_raw_settings_returns_array_when_option_missing() {
+		delete_option( OPTML_NAMESPACE . '_settings' );
+
+		$raw_settings = ( new Optml_Settings() )->get_raw_settings();
+
+		$this->assertIsArray( $raw_settings );
+		$this->assertSame( [], $raw_settings );
+	}
+
+	/**
+	 * Raw settings must be an array even when the option holds a non-array value.
+	 */
+	public function test_get_raw_settings_returns_array_when_option_is_not_an_array() {
+		update_option( OPTML_NAMESPACE . '_settings', 'corrupted' );
+
+		$this->assertSame( [], ( new Optml_Settings() )->get_raw_settings() );
+	}
+
+	/**
+	 * `add_settings` must not fatal when there is nothing stored in the database.
+	 */
+	public function test_add_settings_with_missing_option() {
+		delete_option( OPTML_NAMESPACE . '_settings' );
+
+		$data = Optml_Main::add_settings( [ 'foo' => 'bar' ] );
+
+		$this->assertIsArray( $data );
+		$this->assertSame( [ 'foo' => 'bar' ], $data );
+	}
+
+	/**
+	 * Secrets are never sent along with the logger data.
+	 */
+	public function test_add_settings_strips_secrets() {
+		update_option(
+			OPTML_NAMESPACE . '_settings',
+			[
+				'api_key'      => 'secret-key',
+				'service_data' => [ 'cdn_key' => 'key', 'cdn_secret' => 'secret' ],
+				'quality'      => 'auto',
+			]
+		);
+
+		$data = Optml_Main::add_settings( [] );
+
+		$this->assertArrayNotHasKey( 'api_key', $data );
+		$this->assertArrayNotHasKey( 'service_data', $data );
+		$this->assertSame( 'auto', $data['quality'] );
+	}
+
+	/**
+	 * The logger cron collects its payload through this filter, it must survive a site
+	 * where Optimole settings were never saved.
+	 */
+	public function test_logger_data_filter_without_stored_settings() {
+		delete_option( OPTML_NAMESPACE . '_settings' );
+
+		$this->assertNotFalse( has_filter( 'optimole_wp_logger_data', [ 'Optml_Main', 'add_settings' ] ) );
+		$this->assertIsArray( apply_filters( 'optimole_wp_logger_data', [] ) );
+	}
+}