From f8f27fff138fea0b03121b14fa002459328a620f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 23 Mar 2026 10:37:49 +0100 Subject: [PATCH] refactor(scheduler): extract cron scheduling logic to shared helper Extract the shouldRunNow() method from ScheduledJobManager and ServerManagerJob into a reusable shouldRunCronNow() helper function. This centralizes cron scheduling logic and enables consistent deduplication behavior across all scheduled job types. - Create shouldRunCronNow() helper in bootstrap/helpers/shared.php with timezone and dedup support - Refactor ScheduledJobManager and ServerManagerJob to use the shared helper - Add ScheduledJobDiagnostics command for inspecting cache state and scheduling decisions across all scheduled jobs - Simplify shouldRunNow tests to directly test the helper function - Add DockerCleanupJob test for error handling and execution tracking - Increase scheduled log retention from 1 to 7 days --- .../Commands/ScheduledJobDiagnostics.php | 255 ++++++++++++++++++ app/Jobs/ScheduledJobManager.php | 52 +--- app/Jobs/ServerManagerJob.php | 53 +--- bootstrap/helpers/shared.php | 30 +++ config/logging.php | 2 +- tests/Feature/DockerCleanupJobTest.php | 50 ++++ .../ScheduledJobManagerShouldRunNowTest.php | 229 +++++----------- .../ServerManagerJobShouldRunNowTest.php | 92 +++---- 8 files changed, 446 insertions(+), 317 deletions(-) create mode 100644 app/Console/Commands/ScheduledJobDiagnostics.php create mode 100644 tests/Feature/DockerCleanupJobTest.php diff --git a/app/Console/Commands/ScheduledJobDiagnostics.php b/app/Console/Commands/ScheduledJobDiagnostics.php new file mode 100644 index 000000000..77881284c --- /dev/null +++ b/app/Console/Commands/ScheduledJobDiagnostics.php @@ -0,0 +1,255 @@ +option('type'); + $serverFilter = $this->option('server'); + + $this->outputHeartbeat(); + + if (in_array($type, ['all', 'docker-cleanup'])) { + $this->inspectDockerCleanups($serverFilter); + } + + if (in_array($type, ['all', 'backups'])) { + $this->inspectBackups(); + } + + if (in_array($type, ['all', 'tasks'])) { + $this->inspectTasks(); + } + + if (in_array($type, ['all', 'server-jobs'])) { + $this->inspectServerJobs($serverFilter); + } + + return self::SUCCESS; + } + + private function outputHeartbeat(): void + { + $heartbeat = Cache::get('scheduled-job-manager:heartbeat'); + if ($heartbeat) { + $age = Carbon::parse($heartbeat)->diffForHumans(); + $this->info("Scheduler heartbeat: {$heartbeat} ({$age})"); + } else { + $this->error('Scheduler heartbeat: MISSING — ScheduledJobManager may not be running'); + } + $this->newLine(); + } + + private function inspectDockerCleanups(?string $serverFilter): void + { + $this->info('=== Docker Cleanup Jobs ==='); + + $servers = $this->getServers($serverFilter); + + $rows = []; + foreach ($servers as $server) { + $frequency = data_get($server->settings, 'docker_cleanup_frequency', '0 * * * *'); + if (isset(VALID_CRON_STRINGS[$frequency])) { + $frequency = VALID_CRON_STRINGS[$frequency]; + } + + $dedupKey = "docker-cleanup:{$server->id}"; + $cacheValue = Cache::get($dedupKey); + $timezone = data_get($server->settings, 'server_timezone', config('app.timezone')); + + if (validate_timezone($timezone) === false) { + $timezone = config('app.timezone'); + } + + $wouldFire = shouldRunCronNow($frequency, $timezone, $dedupKey); + + $lastExecution = DockerCleanupExecution::where('server_id', $server->id) + ->latest() + ->first(); + + $rows[] = [ + $server->id, + $server->name, + $timezone, + $frequency, + $dedupKey, + $cacheValue ?? '', + $wouldFire ? 'YES' : 'no', + $lastExecution ? $lastExecution->status.' @ '.$lastExecution->created_at : 'never', + ]; + } + + $this->table( + ['ID', 'Server', 'TZ', 'Frequency', 'Dedup Key', 'Cache Value', 'Would Fire', 'Last Execution'], + $rows + ); + $this->newLine(); + } + + private function inspectBackups(): void + { + $this->info('=== Scheduled Backups ==='); + + $backups = ScheduledDatabaseBackup::with(['database']) + ->where('enabled', true) + ->get(); + + $rows = []; + foreach ($backups as $backup) { + $server = $backup->server(); + $frequency = $backup->frequency; + if (isset(VALID_CRON_STRINGS[$frequency])) { + $frequency = VALID_CRON_STRINGS[$frequency]; + } + + $dedupKey = "scheduled-backup:{$backup->id}"; + $cacheValue = Cache::get($dedupKey); + $timezone = $server ? data_get($server->settings, 'server_timezone', config('app.timezone')) : config('app.timezone'); + + if (validate_timezone($timezone) === false) { + $timezone = config('app.timezone'); + } + + $wouldFire = shouldRunCronNow($frequency, $timezone, $dedupKey); + + $rows[] = [ + $backup->id, + $backup->database_type ?? 'unknown', + $server?->name ?? 'N/A', + $frequency, + $cacheValue ?? '', + $wouldFire ? 'YES' : 'no', + ]; + } + + $this->table( + ['Backup ID', 'DB Type', 'Server', 'Frequency', 'Cache Value', 'Would Fire'], + $rows + ); + $this->newLine(); + } + + private function inspectTasks(): void + { + $this->info('=== Scheduled Tasks ==='); + + $tasks = ScheduledTask::with(['service', 'application']) + ->where('enabled', true) + ->get(); + + $rows = []; + foreach ($tasks as $task) { + $server = $task->server(); + $frequency = $task->frequency; + if (isset(VALID_CRON_STRINGS[$frequency])) { + $frequency = VALID_CRON_STRINGS[$frequency]; + } + + $dedupKey = "scheduled-task:{$task->id}"; + $cacheValue = Cache::get($dedupKey); + $timezone = $server ? data_get($server->settings, 'server_timezone', config('app.timezone')) : config('app.timezone'); + + if (validate_timezone($timezone) === false) { + $timezone = config('app.timezone'); + } + + $wouldFire = shouldRunCronNow($frequency, $timezone, $dedupKey); + + $rows[] = [ + $task->id, + $task->name, + $server?->name ?? 'N/A', + $frequency, + $cacheValue ?? '', + $wouldFire ? 'YES' : 'no', + ]; + } + + $this->table( + ['Task ID', 'Name', 'Server', 'Frequency', 'Cache Value', 'Would Fire'], + $rows + ); + $this->newLine(); + } + + private function inspectServerJobs(?string $serverFilter): void + { + $this->info('=== Server Manager Jobs ==='); + + $servers = $this->getServers($serverFilter); + + $rows = []; + foreach ($servers as $server) { + $timezone = data_get($server->settings, 'server_timezone', config('app.timezone')); + if (validate_timezone($timezone) === false) { + $timezone = config('app.timezone'); + } + + $dedupKeys = [ + "sentinel-restart:{$server->id}" => '0 0 * * *', + "server-patch-check:{$server->id}" => '0 0 * * 0', + "server-check:{$server->id}" => isCloud() ? '*/5 * * * *' : '* * * * *', + "server-storage-check:{$server->id}" => data_get($server->settings, 'server_disk_usage_check_frequency', '0 23 * * *'), + ]; + + foreach ($dedupKeys as $dedupKey => $frequency) { + if (isset(VALID_CRON_STRINGS[$frequency])) { + $frequency = VALID_CRON_STRINGS[$frequency]; + } + + $cacheValue = Cache::get($dedupKey); + $wouldFire = shouldRunCronNow($frequency, $timezone, $dedupKey); + + $rows[] = [ + $server->id, + $server->name, + $dedupKey, + $frequency, + $cacheValue ?? '', + $wouldFire ? 'YES' : 'no', + ]; + } + } + + $this->table( + ['Server ID', 'Server', 'Dedup Key', 'Frequency', 'Cache Value', 'Would Fire'], + $rows + ); + $this->newLine(); + } + + private function getServers(?string $serverFilter): \Illuminate\Support\Collection + { + $query = Server::with('settings')->where('ip', '!=', '1.2.3.4'); + + if ($serverFilter) { + $query->where('id', $serverFilter); + } + + if (isCloud()) { + $servers = $query->whereRelation('team.subscription', 'stripe_invoice_paid', true)->get(); + $own = Team::find(0)?->servers()->with('settings')->get() ?? collect(); + + return $servers->merge($own); + } + + return $query->get(); + } +} diff --git a/app/Jobs/ScheduledJobManager.php b/app/Jobs/ScheduledJobManager.php index ebcd229ed..71829ea41 100644 --- a/app/Jobs/ScheduledJobManager.php +++ b/app/Jobs/ScheduledJobManager.php @@ -6,7 +6,6 @@ use App\Models\ScheduledTask; use App\Models\Server; use App\Models\Team; -use Cron\CronExpression; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; @@ -185,7 +184,7 @@ private function processScheduledBackups(): void $frequency = VALID_CRON_STRINGS[$frequency]; } - if ($this->shouldRunNow($frequency, $serverTimezone, "scheduled-backup:{$backup->id}")) { + if (shouldRunCronNow($frequency, $serverTimezone, "scheduled-backup:{$backup->id}", $this->executionTime)) { DatabaseBackupJob::dispatch($backup); $this->dispatchedCount++; Log::channel('scheduled')->info('Backup dispatched', [ @@ -239,7 +238,7 @@ private function processScheduledTasks(): void $frequency = VALID_CRON_STRINGS[$frequency]; } - if (! $this->shouldRunNow($frequency, $serverTimezone, "scheduled-task:{$task->id}")) { + if (! shouldRunCronNow($frequency, $serverTimezone, "scheduled-task:{$task->id}", $this->executionTime)) { continue; } @@ -336,51 +335,6 @@ private function getTaskRuntimeSkipReason(ScheduledTask $task): ?string return null; } - /** - * Determine if a cron schedule should run now. - * - * When a dedupKey is provided, uses getPreviousRunDate() + last-dispatch tracking - * instead of isDue(). This is resilient to queue delays — even if the job is delayed - * by minutes, it still catches the missed cron window. Without dedupKey, falls back - * to simple isDue() check. - */ - private function shouldRunNow(string $frequency, string $timezone, ?string $dedupKey = null): bool - { - $cron = new CronExpression($frequency); - $baseTime = $this->executionTime ?? Carbon::now(); - $executionTime = $baseTime->copy()->setTimezone($timezone); - - // No dedup key → simple isDue check - if ($dedupKey === null) { - return $cron->isDue($executionTime); - } - - // Get the most recent time this cron was due (including current minute) - $previousDue = Carbon::instance($cron->getPreviousRunDate($executionTime, allowCurrentDate: true)); - - $lastDispatched = Cache::get($dedupKey); - - if ($lastDispatched === null) { - // First run after restart or cache loss: only fire if actually due right now. - // Seed the cache so subsequent runs can use tolerance/catch-up logic. - $isDue = $cron->isDue($executionTime); - if ($isDue) { - Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); - } - - return $isDue; - } - - // Subsequent runs: fire if there's been a due time since last dispatch - if ($previousDue->gt(Carbon::parse($lastDispatched))) { - Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); - - return true; - } - - return false; - } - private function processDockerCleanups(): void { // Get all servers that need cleanup checks @@ -411,7 +365,7 @@ private function processDockerCleanups(): void } // Use the frozen execution time for consistent evaluation - if ($this->shouldRunNow($frequency, $serverTimezone, "docker-cleanup:{$server->id}")) { + if (shouldRunCronNow($frequency, $serverTimezone, "docker-cleanup:{$server->id}", $this->executionTime)) { DockerCleanupJob::dispatch( $server, false, diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index d56ff0a8c..3f748f0ca 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -5,7 +5,6 @@ use App\Models\InstanceSettings; use App\Models\Server; use App\Models\Team; -use Cron\CronExpression; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -14,7 +13,6 @@ use Illuminate\Queue\SerializesModels; use Illuminate\Support\Carbon; use Illuminate\Support\Collection; -use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue @@ -81,7 +79,7 @@ private function getServers(): Collection private function dispatchConnectionChecks(Collection $servers): void { - if ($this->shouldRunNow($this->checkFrequency, dedupKey: 'server-connection-checks')) { + if (shouldRunCronNow($this->checkFrequency, $this->instanceTimezone, 'server-connection-checks', $this->executionTime)) { $servers->each(function (Server $server) { try { // Skip SSH connection check if Sentinel is healthy — its heartbeat already proves connectivity @@ -130,13 +128,13 @@ private function processServerTasks(Server $server): void if ($sentinelOutOfSync) { // Dispatch ServerCheckJob if Sentinel is out of sync - if ($this->shouldRunNow($this->checkFrequency, $serverTimezone, "server-check:{$server->id}")) { + if (shouldRunCronNow($this->checkFrequency, $serverTimezone, "server-check:{$server->id}", $this->executionTime)) { ServerCheckJob::dispatch($server); } } $isSentinelEnabled = $server->isSentinelEnabled(); - $shouldRestartSentinel = $isSentinelEnabled && $this->shouldRunNow('0 0 * * *', $serverTimezone, "sentinel-restart:{$server->id}"); + $shouldRestartSentinel = $isSentinelEnabled && shouldRunCronNow('0 0 * * *', $serverTimezone, "sentinel-restart:{$server->id}", $this->executionTime); // Dispatch Sentinel restart if due (daily for Sentinel-enabled servers) if ($shouldRestartSentinel) { @@ -150,7 +148,7 @@ private function processServerTasks(Server $server): void if (isset(VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency])) { $serverDiskUsageCheckFrequency = VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency]; } - $shouldRunStorageCheck = $this->shouldRunNow($serverDiskUsageCheckFrequency, $serverTimezone, "server-storage-check:{$server->id}"); + $shouldRunStorageCheck = shouldRunCronNow($serverDiskUsageCheckFrequency, $serverTimezone, "server-storage-check:{$server->id}", $this->executionTime); if ($shouldRunStorageCheck) { ServerStorageCheckJob::dispatch($server); @@ -158,7 +156,7 @@ private function processServerTasks(Server $server): void } // Dispatch ServerPatchCheckJob if due (weekly) - $shouldRunPatchCheck = $this->shouldRunNow('0 0 * * 0', $serverTimezone, "server-patch-check:{$server->id}"); + $shouldRunPatchCheck = shouldRunCronNow('0 0 * * 0', $serverTimezone, "server-patch-check:{$server->id}", $this->executionTime); if ($shouldRunPatchCheck) { // Weekly on Sunday at midnight ServerPatchCheckJob::dispatch($server); @@ -167,45 +165,4 @@ private function processServerTasks(Server $server): void // Note: CheckAndStartSentinelJob is only dispatched daily (line above) for version updates. // Crash recovery is handled by sentinelOutOfSync → ServerCheckJob → CheckAndStartSentinelJob. } - - /** - * Determine if a cron schedule should run now. - * - * When a dedupKey is provided, uses getPreviousRunDate() + last-dispatch tracking - * instead of isDue(). This is resilient to queue delays — even if the job is delayed - * by minutes, it still catches the missed cron window. - */ - private function shouldRunNow(string $frequency, ?string $timezone = null, ?string $dedupKey = null): bool - { - $cron = new CronExpression($frequency); - - // Use the frozen execution time, not the current time - $baseTime = $this->executionTime ?? Carbon::now(); - $executionTime = $baseTime->copy()->setTimezone($timezone ?? config('app.timezone')); - - if ($dedupKey === null) { - return $cron->isDue($executionTime); - } - - $previousDue = Carbon::instance($cron->getPreviousRunDate($executionTime, allowCurrentDate: true)); - - $lastDispatched = Cache::get($dedupKey); - - if ($lastDispatched === null) { - $isDue = $cron->isDue($executionTime); - if ($isDue) { - Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); - } - - return $isDue; - } - - if ($previousDue->gt(Carbon::parse($lastDispatched))) { - Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); - - return true; - } - - return false; - } } diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index e81d2a467..26aa21a7b 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -466,6 +466,36 @@ function validate_cron_expression($expression_to_validate): bool return $isValid; } +/** + * Determine if a cron schedule should run now, with deduplication. + * + * Uses getPreviousRunDate() + last-dispatch tracking to be resilient to queue delays. + * Even if the job runs minutes late, it still catches the missed cron window. + * Without a dedupKey, falls back to a simple isDue() check. + */ +function shouldRunCronNow(string $frequency, string $timezone, ?string $dedupKey = null, ?\Illuminate\Support\Carbon $executionTime = null): bool +{ + $cron = new \Cron\CronExpression($frequency); + $executionTime = ($executionTime ?? \Illuminate\Support\Carbon::now())->copy()->setTimezone($timezone); + + if ($dedupKey === null) { + return $cron->isDue($executionTime); + } + + $previousDue = \Illuminate\Support\Carbon::instance($cron->getPreviousRunDate($executionTime, allowCurrentDate: true)); + $lastDispatched = Cache::get($dedupKey); + + $shouldFire = $lastDispatched === null + ? $cron->isDue($executionTime) + : $previousDue->gt(\Illuminate\Support\Carbon::parse($lastDispatched)); + + // Always write: seeds on first miss, refreshes on dispatch. + // 30-day static TTL covers all intervals; orphan keys self-clean. + Cache::put($dedupKey, ($shouldFire ? $executionTime : $previousDue)->toIso8601String(), 2592000); + + return $shouldFire; +} + function validate_timezone(string $timezone): bool { return in_array($timezone, timezone_identifiers_list()); diff --git a/config/logging.php b/config/logging.php index 1a75978f3..1dbb1135f 100644 --- a/config/logging.php +++ b/config/logging.php @@ -123,7 +123,7 @@ 'driver' => 'daily', 'path' => storage_path('logs/scheduled.log'), 'level' => 'debug', - 'days' => 1, + 'days' => 7, ], 'scheduled-errors' => [ diff --git a/tests/Feature/DockerCleanupJobTest.php b/tests/Feature/DockerCleanupJobTest.php new file mode 100644 index 000000000..446260e22 --- /dev/null +++ b/tests/Feature/DockerCleanupJobTest.php @@ -0,0 +1,50 @@ +create(); + $team = $user->teams()->first(); + $server = Server::factory()->create(['team_id' => $team->id]); + + // Make server not functional by setting is_reachable to false + $server->settings->update(['is_reachable' => false]); + + $job = new DockerCleanupJob($server); + $job->handle(); + + $execution = DockerCleanupExecution::where('server_id', $server->id)->first(); + + expect($execution)->not->toBeNull() + ->and($execution->status)->toBe('failed') + ->and($execution->message)->toContain('not functional') + ->and($execution->finished_at)->not->toBeNull(); +}); + +it('creates a failed execution record when server is force disabled', function () { + $user = User::factory()->create(); + $team = $user->teams()->first(); + $server = Server::factory()->create(['team_id' => $team->id]); + + // Make server not functional by force disabling + $server->settings->update([ + 'is_reachable' => true, + 'is_usable' => true, + 'force_disabled' => true, + ]); + + $job = new DockerCleanupJob($server); + $job->handle(); + + $execution = DockerCleanupExecution::where('server_id', $server->id)->first(); + + expect($execution)->not->toBeNull() + ->and($execution->status)->toBe('failed') + ->and($execution->message)->toContain('not functional'); +}); diff --git a/tests/Feature/ScheduledJobManagerShouldRunNowTest.php b/tests/Feature/ScheduledJobManagerShouldRunNowTest.php index e445e9908..84db743fa 100644 --- a/tests/Feature/ScheduledJobManagerShouldRunNowTest.php +++ b/tests/Feature/ScheduledJobManagerShouldRunNowTest.php @@ -1,271 +1,168 @@ getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:1'); + $result = shouldRunCronNow('0 2 * * *', 'UTC', 'test-backup:1'); expect($result)->toBeTrue(); }); it('catches delayed job when cache has a baseline from previous run', function () { - // Simulate a previous dispatch yesterday at 02:00 Cache::put('test-backup:1', Carbon::create(2026, 2, 27, 2, 0, 0, 'UTC')->toIso8601String(), 86400); - // Freeze time at 02:07 — job was delayed 7 minutes past today's 02:00 cron Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 7, 0, 'UTC')); - $job = new ScheduledJobManager; - - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - // isDue() would return false at 02:07, but getPreviousRunDate() = 02:00 today // lastDispatched = 02:00 yesterday → 02:00 today > yesterday → fires - $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:1'); + $result = shouldRunCronNow('0 2 * * *', 'UTC', 'test-backup:1'); expect($result)->toBeTrue(); }); it('does not double-dispatch on subsequent runs within same cron window', function () { - // First run at 02:00 — dispatches and sets cache Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')); - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - $first = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:2'); + $first = shouldRunCronNow('0 2 * * *', 'UTC', 'test-backup:2'); expect($first)->toBeTrue(); // Second run at 02:01 — should NOT dispatch (previousDue=02:00, lastDispatched=02:00) Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 1, 0, 'UTC')); - $executionTimeProp->setValue($job, Carbon::now()); - $second = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:2'); + $second = shouldRunCronNow('0 2 * * *', 'UTC', 'test-backup:2'); expect($second)->toBeFalse(); }); it('fires every_minute cron correctly on consecutive minutes', function () { - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - // Minute 1 Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC')); - $executionTimeProp->setValue($job, Carbon::now()); - $result1 = $method->invoke($job, '* * * * *', 'UTC', 'test-backup:3'); - expect($result1)->toBeTrue(); + expect(shouldRunCronNow('* * * * *', 'UTC', 'test-backup:3'))->toBeTrue(); // Minute 2 Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 1, 0, 'UTC')); - $executionTimeProp->setValue($job, Carbon::now()); - $result2 = $method->invoke($job, '* * * * *', 'UTC', 'test-backup:3'); - expect($result2)->toBeTrue(); + expect(shouldRunCronNow('* * * * *', 'UTC', 'test-backup:3'))->toBeTrue(); // Minute 3 Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 2, 0, 'UTC')); - $executionTimeProp->setValue($job, Carbon::now()); - $result3 = $method->invoke($job, '* * * * *', 'UTC', 'test-backup:3'); - expect($result3)->toBeTrue(); + expect(shouldRunCronNow('* * * * *', 'UTC', 'test-backup:3'))->toBeTrue(); }); it('does not fire non-due jobs on restart when cache is empty', function () { // Time is 10:00, cron is daily at 02:00 — NOT due right now Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC')); - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - // Cache is empty (fresh restart) — should NOT fire daily backup at 10:00 - $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4'); + $result = shouldRunCronNow('0 2 * * *', 'UTC', 'test-backup:4'); expect($result)->toBeFalse(); }); it('fires due jobs on restart when cache is empty', function () { - // Time is exactly 02:00, cron is daily at 02:00 — IS due right now Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')); - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - // Cache is empty (fresh restart) — but cron IS due → should fire - $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:4b'); + $result = shouldRunCronNow('0 2 * * *', 'UTC', 'test-backup:4b'); expect($result)->toBeTrue(); }); it('does not dispatch when cron is not due and was not recently due', function () { - // Time is 10:00, cron is daily at 02:00 — last due was 8 hours ago Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC')); - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - // previousDue = 02:00, but lastDispatched was set at 02:00 (simulate) Cache::put('test-backup:5', Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')->toIso8601String(), 86400); - $result = $method->invoke($job, '0 2 * * *', 'UTC', 'test-backup:5'); + $result = shouldRunCronNow('0 2 * * *', 'UTC', 'test-backup:5'); expect($result)->toBeFalse(); }); it('falls back to isDue when no dedup key is provided', function () { - // Time is exactly 02:00, cron is "0 2 * * *" — should be due Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')); + expect(shouldRunCronNow('0 2 * * *', 'UTC'))->toBeTrue(); - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - // No dedup key → simple isDue check - $result = $method->invoke($job, '0 2 * * *', 'UTC'); - expect($result)->toBeTrue(); - - // At 02:01 without dedup key → isDue returns false Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 1, 0, 'UTC')); - $executionTimeProp->setValue($job, Carbon::now()); - - $result2 = $method->invoke($job, '0 2 * * *', 'UTC'); - expect($result2)->toBeFalse(); + expect(shouldRunCronNow('0 2 * * *', 'UTC'))->toBeFalse(); }); it('catches delayed docker cleanup when job runs past the cron minute', function () { - // Simulate a previous dispatch at :10 Cache::put('docker-cleanup:42', Carbon::create(2026, 2, 28, 10, 10, 0, 'UTC')->toIso8601String(), 86400); - // Freeze time at :22 — job was delayed 2 minutes past the :20 cron window Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 22, 0, 'UTC')); - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - // isDue() would return false at :22, but getPreviousRunDate() = :20 // lastDispatched = :10 → :20 > :10 → fires - $result = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:42'); + $result = shouldRunCronNow('*/10 * * * *', 'UTC', 'docker-cleanup:42'); expect($result)->toBeTrue(); }); it('does not double-dispatch docker cleanup within same cron window', function () { - // First dispatch at :10 Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 10, 0, 'UTC')); - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - $first = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:99'); + $first = shouldRunCronNow('*/10 * * * *', 'UTC', 'docker-cleanup:99'); expect($first)->toBeTrue(); - // Second run at :11 — should NOT dispatch (previousDue=:10, lastDispatched=:10) Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 11, 0, 'UTC')); - $executionTimeProp->setValue($job, Carbon::now()); - $second = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:99'); + $second = shouldRunCronNow('*/10 * * * *', 'UTC', 'docker-cleanup:99'); expect($second)->toBeFalse(); }); +it('seeds cache with previousDue when not due on first run', function () { + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC')); + + $result = shouldRunCronNow('0 2 * * *', 'UTC', 'test-seed:1'); + expect($result)->toBeFalse(); + + // Verify cache was seeded with previousDue (02:00 today) + $cached = Cache::get('test-seed:1'); + expect($cached)->not->toBeNull(); + expect(Carbon::parse($cached)->format('H:i'))->toBe('02:00'); +}); + +it('catches next occurrence after cache was seeded on non-due first run', function () { + // Step 1: 10:00 — not due, but seeds cache with previousDue (02:00 today) + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 0, 0, 'UTC')); + expect(shouldRunCronNow('0 2 * * *', 'UTC', 'test-seed:2'))->toBeFalse(); + + // Step 2: Next day at 02:03 — delayed 3 minutes past cron. + // previousDue = 02:00 Mar 1, lastDispatched = 02:00 Feb 28 → fires + Carbon::setTestNow(Carbon::create(2026, 3, 1, 2, 3, 0, 'UTC')); + expect(shouldRunCronNow('0 2 * * *', 'UTC', 'test-seed:2'))->toBeTrue(); +}); + +it('cache survives 29 days with static 30-day TTL', function () { + Carbon::setTestNow(Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC')); + + shouldRunCronNow('0 2 * * *', 'UTC', 'test-ttl:static'); + expect(Cache::get('test-ttl:static'))->not->toBeNull(); + + // 29 days later — cache (30-day TTL) should still exist + Carbon::setTestNow(Carbon::create(2026, 3, 29, 0, 0, 0, 'UTC')); + expect(Cache::get('test-ttl:static'))->not->toBeNull(); +}); + it('respects server timezone for cron evaluation', function () { // UTC time is 22:00 Feb 28, which is 06:00 Mar 1 in Asia/Singapore (+8) Carbon::setTestNow(Carbon::create(2026, 2, 28, 22, 0, 0, 'UTC')); - $job = new ScheduledJobManager; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - // Simulate that today's 06:00 UTC run was already dispatched (at 06:00 UTC) Cache::put('test-backup:7', Carbon::create(2026, 2, 28, 6, 0, 0, 'UTC')->toIso8601String(), 86400); - // Cron "0 6 * * *" in Asia/Singapore: local time is 06:00 Mar 1 → previousDue = 06:00 Mar 1 SGT - // That's a NEW cron window (Mar 1) that hasn't been dispatched → should fire - $resultSingapore = $method->invoke($job, '0 6 * * *', 'Asia/Singapore', 'test-backup:6'); - expect($resultSingapore)->toBeTrue(); + // Cron "0 6 * * *" in Asia/Singapore: local time is 06:00 Mar 1 → new window → should fire + expect(shouldRunCronNow('0 6 * * *', 'Asia/Singapore', 'test-backup:6'))->toBeTrue(); - // Cron "0 6 * * *" in UTC: previousDue = 06:00 Feb 28 UTC, already dispatched at 06:00 → should NOT fire - $resultUtc = $method->invoke($job, '0 6 * * *', 'UTC', 'test-backup:7'); - expect($resultUtc)->toBeFalse(); + // Cron "0 6 * * *" in UTC: previousDue = 06:00 Feb 28, already dispatched → should NOT fire + expect(shouldRunCronNow('0 6 * * *', 'UTC', 'test-backup:7'))->toBeFalse(); +}); + +it('passes explicit execution time instead of using Carbon::now()', function () { + // Real "now" is irrelevant — we pass an explicit execution time + Carbon::setTestNow(Carbon::create(2026, 2, 28, 15, 0, 0, 'UTC')); + + $executionTime = Carbon::create(2026, 2, 28, 2, 0, 0, 'UTC'); + $result = shouldRunCronNow('0 2 * * *', 'UTC', 'test-exec-time:1', $executionTime); + + expect($result)->toBeTrue(); }); diff --git a/tests/Feature/ServerManagerJobShouldRunNowTest.php b/tests/Feature/ServerManagerJobShouldRunNowTest.php index 518f05c9c..2743a8650 100644 --- a/tests/Feature/ServerManagerJobShouldRunNowTest.php +++ b/tests/Feature/ServerManagerJobShouldRunNowTest.php @@ -1,6 +1,5 @@ toIso8601String(), 86400); // Job runs 3 minutes late at 00:03 Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 3, 0, 'UTC')); - $job = new ServerManagerJob; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - // isDue() would return false at 00:03, but getPreviousRunDate() = 00:00 today // lastDispatched = yesterday 00:00 → today 00:00 > yesterday → fires - $result = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:1'); + $result = shouldRunCronNow('0 0 * * *', 'UTC', 'sentinel-restart:1'); expect($result)->toBeTrue(); }); it('catches delayed weekly patch check when job runs past the cron minute', function () { - // Simulate previous dispatch last Sunday at midnight Cache::put('server-patch-check:1', Carbon::create(2026, 2, 22, 0, 0, 0, 'UTC')->toIso8601String(), 86400); - // This Sunday at 00:02 — job was delayed 2 minutes - // 2026-03-01 is a Sunday + // This Sunday at 00:02 — job was delayed 2 minutes (2026-03-01 is a Sunday) Carbon::setTestNow(Carbon::create(2026, 3, 1, 0, 2, 0, 'UTC')); - $job = new ServerManagerJob; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - $result = $method->invoke($job, '0 0 * * 0', 'UTC', 'server-patch-check:1'); + $result = shouldRunCronNow('0 0 * * 0', 'UTC', 'server-patch-check:1'); expect($result)->toBeTrue(); }); it('catches delayed storage check when job runs past the cron minute', function () { - // Simulate previous dispatch yesterday at 23:00 Cache::put('server-storage-check:5', Carbon::create(2026, 2, 27, 23, 0, 0, 'UTC')->toIso8601String(), 86400); - // Today at 23:04 — job was delayed 4 minutes Carbon::setTestNow(Carbon::create(2026, 2, 28, 23, 4, 0, 'UTC')); - $job = new ServerManagerJob; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - $result = $method->invoke($job, '0 23 * * *', 'UTC', 'server-storage-check:5'); + $result = shouldRunCronNow('0 23 * * *', 'UTC', 'server-storage-check:5'); expect($result)->toBeTrue(); }); +it('seeds cache on non-due first run so weekly catch-up works', function () { + // Wednesday at 10:00 — weekly cron (Sunday 00:00) is not due + Carbon::setTestNow(Carbon::create(2026, 2, 25, 10, 0, 0, 'UTC')); + + $result = shouldRunCronNow('0 0 * * 0', 'UTC', 'server-patch-check:seed-test'); + expect($result)->toBeFalse(); + + // Verify cache was seeded + expect(Cache::get('server-patch-check:seed-test'))->not->toBeNull(); + + // Next Sunday at 00:02 — delayed 2 minutes past cron + // Catch-up: previousDue = Mar 1 00:00, lastDispatched = Feb 22 → fires + Carbon::setTestNow(Carbon::create(2026, 3, 1, 0, 2, 0, 'UTC')); + + $result2 = shouldRunCronNow('0 0 * * 0', 'UTC', 'server-patch-check:seed-test'); + expect($result2)->toBeTrue(); +}); + +it('daily cron fires after cache seed even when delayed past the minute', function () { + // Step 1: 15:00 — not due for midnight cron, but seeds cache + Carbon::setTestNow(Carbon::create(2026, 2, 28, 15, 0, 0, 'UTC')); + + $result1 = shouldRunCronNow('0 0 * * *', 'UTC', 'sentinel-restart:seed-test'); + expect($result1)->toBeFalse(); + + // Step 2: Next day at 00:05 — delayed 5 minutes past midnight + // Catch-up: previousDue = Mar 1 00:00, lastDispatched = Feb 28 00:00 → fires + Carbon::setTestNow(Carbon::create(2026, 3, 1, 0, 5, 0, 'UTC')); + + $result2 = shouldRunCronNow('0 0 * * *', 'UTC', 'sentinel-restart:seed-test'); + expect($result2)->toBeTrue(); +}); + it('does not double-dispatch within same cron window', function () { Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 0, 0, 'UTC')); - $job = new ServerManagerJob; - $reflection = new ReflectionClass($job); - - $executionTimeProp = $reflection->getProperty('executionTime'); - $executionTimeProp->setAccessible(true); - $executionTimeProp->setValue($job, Carbon::now()); - - $method = $reflection->getMethod('shouldRunNow'); - $method->setAccessible(true); - - $first = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:10'); + $first = shouldRunCronNow('0 0 * * *', 'UTC', 'sentinel-restart:10'); expect($first)->toBeTrue(); // Next minute — should NOT dispatch again Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 1, 0, 'UTC')); - $executionTimeProp->setValue($job, Carbon::now()); - $second = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:10'); + $second = shouldRunCronNow('0 0 * * *', 'UTC', 'sentinel-restart:10'); expect($second)->toBeFalse(); });