fix: recover the database refresh chain a killed run ends

classes/Visualizer/Module/Setup.php

@@ -498,37 +498,54 @@ private function schedule_refresh_db_action(): void {
 		) {
 			$next = as_next_scheduled_action( $hook, array(), $group );
 			if ( false === $next ) {
-				as_schedule_recurring_action( $timestamp, $interval, $hook, array(), $group );
+				// Unique: this runs on every request, and Action Scheduler only creates the next
+				// recurrence once the current one completes, so a concurrent request can arrive
+				// while nothing is pending. Uniqueness is enforced in the insert itself.
+				as_schedule_recurring_action( $timestamp, $interval, $hook, array(), $group, true );
+			}
+
+			// as_schedule_recurring_action() returns 0 and stores nothing when creation fails,
+			// so drop the WP-Cron fallback only once the action is really there to replace it.
+			if ( false !== as_next_scheduled_action( $hook, array(), $group ) ) {
+				wp_clear_scheduled_hook( $hook );
+				return;
 			}
-			wp_clear_scheduled_hook( $hook );
-			return;
 		}
 
-		wp_clear_scheduled_hook( $hook );
-		wp_schedule_event( $timestamp, $interval_key, $hook );
+		// Re-arm only what is missing or stale: this runs on every request while Action
+		// Scheduler keeps refusing, and resetting a live event would keep the refresh due.
+		$event = wp_get_scheduled_event( $hook );
+		if ( ! $event || $event->schedule !== $interval_key ) {
+			wp_clear_scheduled_hook( $hook );
+			wp_schedule_event( $timestamp, $interval_key, $hook );
+		}
 	}
 
 	/**
-	 * Keep the DB refresh scheduled when Action Scheduler is not available.
+	 * Keep the DB refresh scheduled on whichever scheduler the site can use.
 	 *
-	 * The migration to Action Scheduler clears the WP-Cron event, so a site that
-	 * already migrated and then lost the library would have nothing left running
-	 * the refresh. Re-arms WP-Cron in that case; no-op whenever the library is up.
+	 * This is the only way back: once the refresh hook has no trigger, nothing but a
+	 * reactivation used to restore it, and reactivation can fail the same way. Also covers
+	 * a site that lost Action Scheduler, and a legacy WP-Cron event that never migrated.
+	 *
+	 * ponytail: two indexed queries per request; cache in a transient if a profile ever complains.
 	 */
 	public function maybe_reschedule_refresh_db(): void {
+		$hook = 'visualizer_schedule_refresh_db';
+
 		if (
 			visualizer_can_use_action_scheduler()
 			&& function_exists( 'as_next_scheduled_action' )
 			&& function_exists( 'as_schedule_recurring_action' )
 		) {
-			return;
+			$scheduled = false !== as_next_scheduled_action( $hook, array(), 'visualizer' );
+		} else {
+			$scheduled = (bool) wp_next_scheduled( $hook );
 		}
 
-		if ( wp_next_scheduled( 'visualizer_schedule_refresh_db' ) ) {
-			return;
+		if ( ! $scheduled ) {
+			$this->schedule_refresh_db_action();
 		}
-
-		$this->schedule_refresh_db_action();
 	}
 
 	/**

classes/Visualizer/Module/Upgrade.php

@@ -24,11 +24,6 @@ public static function upgrade() {
 			$upgraded = true;
 		}
 
-		if ( wp_next_scheduled( 'visualizer_schedule_refresh_db' ) ) {
-			self::migrate_action_scheduler();
-			$upgraded = true;
-		}
-
 		if ( ! $upgraded ) {
 			return;
 		}
@@ -79,41 +74,4 @@ private static function makeAllTableChartsTabular() {
 		);
 		// phpcs:enable WordPress.DB.PreparedSQL.NotPrepared
 	}
-
-	/**
-	 * Migrate recurring WP-Cron jobs to Action Scheduler.
-	 */
-	private static function migrate_action_scheduler(): void {
-		if ( ! function_exists( 'as_schedule_recurring_action' ) || ! function_exists( 'as_next_scheduled_action' ) ) {
-			return;
-		}
-
-		$hook         = 'visualizer_schedule_refresh_db';
-		$group        = 'visualizer';
-		$interval_key = apply_filters( 'visualizer_chart_schedule_interval', 'visualizer_ten_minutes' );
-		$interval     = self::get_schedule_interval_seconds( $interval_key );
-		$timestamp    = strtotime( 'midnight' ) - get_option( 'gmt_offset' ) * HOUR_IN_SECONDS;
-
-		$next = as_next_scheduled_action( $hook, array(), $group );
-		if ( false === $next ) {
-			as_schedule_recurring_action( $timestamp, $interval, $hook, array(), $group );
-		}
-
-		wp_clear_scheduled_hook( $hook );
-	}
-
-	/**
-	 * Resolve a cron schedule key to seconds.
-	 *
-	 * @param string $interval_key Cron schedule key.
-	 * @return int Interval in seconds.
-	 */
-	private static function get_schedule_interval_seconds( $interval_key ) {
-		$schedules = wp_get_schedules();
-		if ( isset( $schedules[ $interval_key ]['interval'] ) ) {
-			return (int) $schedules[ $interval_key ]['interval'];
-		}
-
-		return 600;
-	}
 }

tests/test-schedule-refresh-db.php

@@ -0,0 +1,166 @@
+<?php
+/**
+ * Tests for the recurring DB refresh trigger (regression for #1384).
+ *
+ * @package     visualizer
+ * @subpackage  Tests
+ * @license     http://opensource.org/licenses/gpl-2.0.php GNU Public License
+ */
+
+/**
+ * Database charts refresh only while `visualizer_schedule_refresh_db` has a live trigger.
+ *
+ * Action Scheduler returns 0 instead of throwing when it cannot create an action, so the
+ * plugin used to drop the WP-Cron fallback for an action that was never stored, leaving
+ * nothing scheduled and no way back.
+ */
+class Test_Visualizer_Schedule_Refresh_Db extends WP_UnitTestCase {
+
+	const HOOK  = 'visualizer_schedule_refresh_db';
+	const GROUP = 'visualizer';
+
+	/**
+	 * Start every test with neither scheduler armed; the bootstrap activates the plugin.
+	 */
+	public function set_up() {
+		parent::set_up();
+		as_unschedule_all_actions( self::HOOK, array(), self::GROUP );
+		wp_clear_scheduled_hook( self::HOOK );
+	}
+
+	/**
+	 * Whether anything will fire the refresh hook again.
+	 *
+	 * @return bool
+	 */
+	private function has_trigger(): bool {
+		return false !== as_next_scheduled_action( self::HOOK, array(), self::GROUP )
+			|| false !== wp_next_scheduled( self::HOOK );
+	}
+
+	/**
+	 * Fire a plugin lifecycle hook the way WordPress does.
+	 *
+	 * @param string $action Either `activate` or `deactivate`.
+	 */
+	private function lifecycle( string $action ) {
+		do_action( $action . '_' . plugin_basename( VISUALIZER_BASEFILE ), false );
+	}
+
+	/**
+	 * Precondition: Action Scheduler is usable, otherwise the rest proves nothing.
+	 */
+	public function test_action_scheduler_is_available() {
+		$this->assertTrue( function_exists( 'as_schedule_recurring_action' ), 'Action Scheduler must be loaded' );
+		$this->assertTrue( ActionScheduler::is_initialized(), 'Action Scheduler must be initialized' );
+	}
+
+	/**
+	 * A refused Action Scheduler creation must leave the WP-Cron fallback in place.
+	 */
+	public function test_activation_keeps_a_trigger_when_action_scheduler_creation_fails() {
+		add_filter( 'pre_as_schedule_recurring_action', '__return_zero' );
+		$this->lifecycle( 'activate' );
+		remove_filter( 'pre_as_schedule_recurring_action', '__return_zero' );
+
+		$this->assertTrue( $this->has_trigger(), 'activation must not leave the refresh hook with no trigger at all' );
+	}
+
+	/**
+	 * A site that already lost both schedulers must recover on an ordinary request.
+	 */
+	public function test_recovery_restores_a_trigger_when_both_schedulers_are_empty() {
+		$this->assertFalse( $this->has_trigger(), 'precondition: nothing is scheduled' );
+
+		$this->setup_module()->maybe_reschedule_refresh_db();
+
+		$this->assertTrue( $this->has_trigger(), 'a missing refresh trigger must be restored without another activation' );
+	}
+
+	/**
+	 * A legacy WP-Cron event must move onto Action Scheduler and stop firing twice.
+	 */
+	public function test_legacy_wp_cron_event_migrates_to_action_scheduler() {
+		wp_schedule_event( time(), 'visualizer_ten_minutes', self::HOOK );
+		$this->assertNotFalse( wp_next_scheduled( self::HOOK ), 'precondition: a legacy WP-Cron event exists' );
+
+		$this->setup_module()->maybe_reschedule_refresh_db();
+
+		$this->assertNotFalse( as_next_scheduled_action( self::HOOK, array(), self::GROUP ), 'the refresh must move onto Action Scheduler' );
+		$this->assertFalse( wp_next_scheduled( self::HOOK ), 'the superseded WP-Cron event must not survive the migration' );
+	}
+
+	/**
+	 * While Action Scheduler keeps refusing, later requests must leave the fallback alone.
+	 *
+	 * Re-arming it on every request pins the event to a past timestamp, so the refresh runs
+	 * on every cron spawn instead of every ten minutes.
+	 */
+	public function test_recovery_does_not_drag_a_live_wp_cron_event_back_into_the_past() {
+		add_filter( 'pre_as_schedule_recurring_action', '__return_zero' );
+
+		$this->setup_module()->maybe_reschedule_refresh_db();
+		$this->assertNotFalse( wp_next_scheduled( self::HOOK ), 'precondition: the fallback is armed' );
+
+		// mimic WP-Cron having run the event and rescheduled it forward.
+		wp_clear_scheduled_hook( self::HOOK );
+		$future = time() + 600;
+		wp_schedule_event( $future, 'visualizer_ten_minutes', self::HOOK );
+
+		$this->setup_module()->maybe_reschedule_refresh_db();
+		remove_filter( 'pre_as_schedule_recurring_action', '__return_zero' );
+
+		$this->assertSame( $future, wp_next_scheduled( self::HOOK ), 'a later request must not make the refresh due again' );
+	}
+
+	/**
+	 * A concurrent request must not be able to create a second recurring action.
+	 *
+	 * Action Scheduler creates the next recurrence only after the current one completes, so
+	 * every interval there is a moment with nothing pending. A visitor arriving in that
+	 * window used to add a duplicate, and duplicates never go away on their own.
+	 */
+	public function test_recovery_does_not_create_a_second_action_when_a_concurrent_request_wins_the_race() {
+		$done = false;
+		// Stand in for another request that schedules between our lookup and our write.
+		$racer = function ( $pre, $timestamp, $interval, $hook, $args, $group, $priority, $unique ) use ( &$done ) {
+			if ( ! $done ) {
+				$done = true;
+				as_schedule_recurring_action( $timestamp, $interval, $hook, $args, $group, $unique );
+			}
+			return null;
+		};
+		add_filter( 'pre_as_schedule_recurring_action', $racer, 10, 9 );
+
+		$this->setup_module()->maybe_reschedule_refresh_db();
+		remove_filter( 'pre_as_schedule_recurring_action', $racer, 10 );
+
+		$this->assertTrue( $done, 'precondition: the race was actually simulated' );
+		$this->assertCount( 1, $this->pending_actions(), 'a lost race must not leave the refresh scheduled twice' );
+	}
+
+	/**
+	 * Every pending refresh action.
+	 *
+	 * @return array
+	 */
+	private function pending_actions(): array {
+		return as_get_scheduled_actions(
+			array(
+				'hook'   => self::HOOK,
+				'group'  => self::GROUP,
+				'status' => ActionScheduler_Store::STATUS_PENDING,
+			),
+			'ids'
+		);
+	}
+
+	/**
+	 * The registered setup module.
+	 *
+	 * @return Visualizer_Module_Setup
+	 */
+	private function setup_module(): Visualizer_Module_Setup {
+		return Visualizer_Plugin::instance()->getModule( Visualizer_Module_Setup::NAME );
+	}
+}