diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index a618647eb..61266473b 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -120,7 +120,7 @@ private function processServerTasks(Server $server): void // Check if we should run sentinel-based checks $lastSentinelUpdate = $server->sentinel_updated_at; $waitTime = $server->waitBeforeDoingSshCheck(); - $sentinelOutOfSync = Carbon::parse($lastSentinelUpdate)->isBefore($this->executionTime->subSeconds($waitTime)); + $sentinelOutOfSync = Carbon::parse($lastSentinelUpdate)->isBefore($this->executionTime->copy()->subSeconds($waitTime)); if ($sentinelOutOfSync) { // Dispatch ServerCheckJob if Sentinel is out of sync diff --git a/tests/Unit/ServerManagerJobExecutionTimeTest.php b/tests/Unit/ServerManagerJobExecutionTimeTest.php new file mode 100644 index 000000000..d32db6834 --- /dev/null +++ b/tests/Unit/ServerManagerJobExecutionTimeTest.php @@ -0,0 +1,97 @@ +toDateTimeString(); + + // Simulate what happens in processServerTasks() with the FIX applied + $waitTime = 360; + $threshold = $originalTime->copy()->subSeconds($waitTime); + + // The original time should remain unchanged after using copy() + expect($originalTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold->toDateTimeString())->toBe('2024-12-02 11:54:00'); +}); + +it('demonstrates mutation bug when not using copy()', function () { + // This test shows what would happen WITHOUT the fix (the bug) + $originalTime = Carbon::parse('2024-12-02 12:00:00'); + + // Simulate what would happen WITHOUT copy() (the bug) + $waitTime = 360; + $threshold = $originalTime->subSeconds($waitTime); + + // Without copy(), the original time is mutated! + expect($originalTime->toDateTimeString())->toBe('2024-12-02 11:54:00'); + expect($threshold->toDateTimeString())->toBe('2024-12-02 11:54:00'); + expect($originalTime)->toBe($threshold); // They're the same object +}); + +it('preserves executionTime across multiple subSeconds calls with copy()', function () { + // Simulate processing multiple servers with different wait times + $executionTime = Carbon::parse('2024-12-02 12:00:00'); + $originalTimeString = $executionTime->toDateTimeString(); + + // Server 1: waitTime = 360s + $threshold1 = $executionTime->copy()->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold1->toDateTimeString())->toBe('2024-12-02 11:54:00'); + + // Server 2: waitTime = 300s (should still use original time) + $threshold2 = $executionTime->copy()->subSeconds(300); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold2->toDateTimeString())->toBe('2024-12-02 11:55:00'); + + // Server 3: waitTime = 360s (should still use original time) + $threshold3 = $executionTime->copy()->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold3->toDateTimeString())->toBe('2024-12-02 11:54:00'); + + // Server 4: waitTime = 300s (should still use original time) + $threshold4 = $executionTime->copy()->subSeconds(300); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold4->toDateTimeString())->toBe('2024-12-02 11:55:00'); + + // Server 5: waitTime = 360s (should still use original time) + $threshold5 = $executionTime->copy()->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold5->toDateTimeString())->toBe('2024-12-02 11:54:00'); + + // The executionTime should STILL be exactly the original time + expect($executionTime->toDateTimeString())->toBe('2024-12-02 12:00:00'); +}); + +it('demonstrates compounding bug without copy() across multiple calls', function () { + // This shows the compounding bug that happens WITHOUT the fix + $executionTime = Carbon::parse('2024-12-02 12:00:00'); + + // Server 1: waitTime = 360s + $threshold1 = $executionTime->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:54:00'); // MUTATED! + + // Server 2: waitTime = 300s (uses already-mutated time) + $threshold2 = $executionTime->subSeconds(300); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:49:00'); // Further mutated! + + // Server 3: waitTime = 360s (uses even more mutated time) + $threshold3 = $executionTime->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:43:00'); // Even more mutated! + + // Server 4: waitTime = 300s + $threshold4 = $executionTime->subSeconds(300); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:38:00'); + + // Server 5: waitTime = 360s + $threshold5 = $executionTime->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:32:00'); + + // The executionTime is now 1680 seconds (28 minutes) earlier than it should be! + expect($executionTime->diffInSeconds(Carbon::parse('2024-12-02 12:00:00')))->toEqual(1680); +});