Changes to job running to clean up code

Author: DaneEveritt

app/Jobs/Schedule/RunTaskJob.php

@@ -3,7 +3,7 @@
 namespace Pterodactyl\Jobs\Schedule;
 
 use Exception;
-use Carbon\Carbon;
+use Cake\Chronos\Chronos;
 use Pterodactyl\Jobs\Job;
 use InvalidArgumentException;
 use Illuminate\Queue\SerializesModels;
@@ -158,7 +158,7 @@ private function markScheduleComplete()
         $repository = app()->make(ScheduleRepositoryInterface::class);
         $repository->withoutFreshModel()->update($this->schedule, [
             'is_processing' => false,
-            'last_run_at' => Carbon::now()->toDateTimeString(),
+            'last_run_at' => Chronos::now()->toDateTimeString(),
         ]);
     }
 

app/Services/Schedules/ProcessScheduleService.php

@@ -7,36 +7,48 @@
 use Cake\Chronos\Chronos;
 use Pterodactyl\Models\Schedule;
 use Cake\Chronos\ChronosInterface;
-use Pterodactyl\Services\Schedules\Tasks\RunTaskService;
+use Illuminate\Contracts\Bus\Dispatcher;
+use Pterodactyl\Jobs\Schedule\RunTaskJob;
+use Pterodactyl\Contracts\Repository\TaskRepositoryInterface;
 use Pterodactyl\Contracts\Repository\ScheduleRepositoryInterface;
 
 class ProcessScheduleService
 {
     /**
-     * @var \Pterodactyl\Contracts\Repository\ScheduleRepositoryInterface
+     * @var \Illuminate\Contracts\Bus\Dispatcher
      */
-    private $repository;
+    private $dispatcher;
 
     /**
-     * @var \Pterodactyl\Services\Schedules\Tasks\RunTaskService
+     * @var \DateTimeInterface|null
      */
-    private $runnerService;
+    private $runTimeOverride;
 
     /**
-     * @var \DateTimeInterface|null
+     * @var \Pterodactyl\Contracts\Repository\ScheduleRepositoryInterface
      */
-    private $runTimeOverride;
+    private $scheduleRepository;
+
+    /**
+     * @var \Pterodactyl\Contracts\Repository\TaskRepositoryInterface
+     */
+    private $taskRepository;
 
     /**
      * ProcessScheduleService constructor.
      *
-     * @param \Pterodactyl\Services\Schedules\Tasks\RunTaskService          $runnerService
-     * @param \Pterodactyl\Contracts\Repository\ScheduleRepositoryInterface $repository
+     * @param \Illuminate\Contracts\Bus\Dispatcher                          $dispatcher
+     * @param \Pterodactyl\Contracts\Repository\ScheduleRepositoryInterface $scheduleRepository
+     * @param \Pterodactyl\Contracts\Repository\TaskRepositoryInterface     $taskRepository
      */
-    public function __construct(RunTaskService $runnerService, ScheduleRepositoryInterface $repository)
-    {
-        $this->repository = $repository;
-        $this->runnerService = $runnerService;
+    public function __construct(
+        Dispatcher $dispatcher,
+        ScheduleRepositoryInterface $scheduleRepository,
+        TaskRepositoryInterface $taskRepository
+    ) {
+        $this->dispatcher = $dispatcher;
+        $this->scheduleRepository = $scheduleRepository;
+        $this->taskRepository = $taskRepository;
     }
 
     /**
@@ -63,7 +75,10 @@ public function setRunTimeOverride(DateTimeInterface $time)
      */
     public function handle(Schedule $schedule)
     {
-        $this->repository->loadTasks($schedule);
+        $this->scheduleRepository->loadTasks($schedule);
+
+        /** @var \Pterodactyl\Models\Task $task */
+        $task = $schedule->getRelation('tasks')->where('sequence_id', 1)->first();
 
         $formattedCron = sprintf('%s %s %s * %s',
             $schedule->cron_minute,
@@ -72,13 +87,16 @@ public function handle(Schedule $schedule)
             $schedule->cron_day_of_week
         );
 
-        $this->repository->update($schedule->id, [
+        $this->scheduleRepository->update($schedule->id, [
             'is_processing' => true,
             'next_run_at' => $this->getRunAtTime($formattedCron),
         ]);
 
-        $task = $schedule->getRelation('tasks')->where('sequence_id', 1)->first();
-        $this->runnerService->handle($task);
+        $this->taskRepository->update($task->id, ['is_queued' => true]);
+
+        $this->dispatcher->dispatch(
+            (new RunTaskJob($task->id, $schedule->id))->delay($task->time_offset)
+        );
     }
 
     /**

app/Services/Schedules/Tasks/RunTaskService.php

@@ -3,8 +3,8 @@
 namespace Tests\Unit\Jobs\Schedule;
 
 use Mockery as m;
-use Carbon\Carbon;
 use Tests\TestCase;
+use Cake\Chronos\Chronos;
 use Pterodactyl\Models\Task;
 use Pterodactyl\Models\User;
 use GuzzleHttp\Psr7\Response;
@@ -58,7 +58,7 @@ public function setUp()
     {
         parent::setUp();
         Bus::fake();
-        Carbon::setTestNow(Carbon::now());
+        Chronos::setTestNow(Chronos::now());
 
         $this->commandRepository = m::mock(CommandRepositoryInterface::class);
         $this->config = m::mock(Repository::class);
@@ -94,7 +94,7 @@ public function testPowerAction()
 
         $this->scheduleRepository->shouldReceive('withoutFreshModel->update')->with($schedule->id, [
             'is_processing' => false,
-            'last_run_at' => Carbon::now()->toDateTimeString(),
+            'last_run_at' => Chronos::now()->toDateTimeString(),
         ])->once()->andReturnNull();
 
         $this->getJobInstance($task->id, $schedule->id);
@@ -124,7 +124,7 @@ public function testCommandAction()
 
         $this->scheduleRepository->shouldReceive('withoutFreshModel->update')->with($schedule->id, [
             'is_processing' => false,
-            'last_run_at' => Carbon::now()->toDateTimeString(),
+            'last_run_at' => Chronos::now()->toDateTimeString(),
         ])->once()->andReturnNull();
 
         $this->getJobInstance($task->id, $schedule->id);
@@ -202,7 +202,7 @@ public function testScheduleMarkedAsDisabledDoesNotProcess()
 
         $this->scheduleRepository->shouldReceive('withoutFreshModel->update')->with($schedule->id, [
             'is_processing' => false,
-            'last_run_at' => Carbon::now()->toDateTimeString(),
+            'last_run_at' => Chronos::now()->toDateTimeString(),
         ])->once()->andReturn(1);
 
         $this->taskRepository->shouldReceive('update')->with($task->id, ['is_queued' => false])->once()->andReturn(1);

tests/Unit/Jobs/Schedule/RunTaskJobTest.php

@@ -8,31 +8,28 @@
 use Cake\Chronos\Chronos;
 use Pterodactyl\Models\Task;
 use Pterodactyl\Models\Schedule;
-use Pterodactyl\Services\Schedules\Tasks\RunTaskService;
+use Illuminate\Contracts\Bus\Dispatcher;
+use Pterodactyl\Jobs\Schedule\RunTaskJob;
 use Pterodactyl\Services\Schedules\ProcessScheduleService;
+use Pterodactyl\Contracts\Repository\TaskRepositoryInterface;
 use Pterodactyl\Contracts\Repository\ScheduleRepositoryInterface;
 
 class ProcessScheduleServiceTest extends TestCase
 {
     /**
-     * @var \Cron\CronExpression|\Mockery\Mock
+     * @var \Illuminate\Contracts\Bus\Dispatcher|\Mockery\Mock
      */
-    protected $cron;
+    private $dispatcher;
 
     /**
      * @var \Pterodactyl\Contracts\Repository\ScheduleRepositoryInterface|\Mockery\Mock
      */
-    protected $repository;
+    private $scheduleRepository;
 
     /**
-     * @var \Pterodactyl\Services\Schedules\Tasks\RunTaskService|\Mockery\Mock
+     * @var \Pterodactyl\Contracts\Repository\TaskRepositoryInterface|\Mockery\Mock
      */
-    protected $runnerService;
-
-    /**
-     * @var \Pterodactyl\Services\Schedules\ProcessScheduleService
-     */
-    protected $service;
+    private $taskRepository;
 
     /**
      * Setup tests.
@@ -42,11 +39,9 @@ public function setUp()
         parent::setUp();
 
         Chronos::setTestNow(Chronos::now());
-
-        $this->repository = m::mock(ScheduleRepositoryInterface::class);
-        $this->runnerService = m::mock(RunTaskService::class);
-
-        $this->service = new ProcessScheduleService($this->runnerService, $this->repository);
+        $this->dispatcher = m::mock(Dispatcher::class);
+        $this->scheduleRepository = m::mock(ScheduleRepositoryInterface::class);
+        $this->taskRepository = m::mock(TaskRepositoryInterface::class);
     }
 
     /**
@@ -59,17 +54,26 @@ public function testScheduleIsUpdatedAndRun()
             'sequence_id' => 1,
         ])]));
 
-        $this->repository->shouldReceive('loadTasks')->with($model)->once()->andReturn($model);
+        $this->scheduleRepository->shouldReceive('loadTasks')->with($model)->once()->andReturn($model);
 
         $formatted = sprintf('%s %s %s * %s', $model->cron_minute, $model->cron_hour, $model->cron_day_of_month, $model->cron_day_of_week);
-        $this->repository->shouldReceive('update')->with($model->id, [
+        $this->scheduleRepository->shouldReceive('update')->with($model->id, [
             'is_processing' => true,
             'next_run_at' => Chronos::parse(CronExpression::factory($formatted)->getNextRunDate()->format(Chronos::ATOM)),
         ]);
 
-        $this->runnerService->shouldReceive('handle')->with($task)->once()->andReturnNull();
+        $this->taskRepository->shouldReceive('update')->with($task->id, ['is_queued' => true])->once();
+
+        $this->dispatcher->shouldReceive('dispatch')->with(m::on(function ($class) use ($model, $task) {
+            $this->assertInstanceOf(RunTaskJob::class, $class);
+            $this->assertSame($task->time_offset, $class->delay);
+            $this->assertSame($task->id, $class->task);
+            $this->assertSame($model->id, $class->schedule);
 
-        $this->service->handle($model);
+            return true;
+        }))->once();
+
+        $this->getService()->handle($model);
         $this->assertTrue(true);
     }
 
@@ -80,16 +84,30 @@ public function testScheduleRunTimeCanBeOverridden()
             'sequence_id' => 1,
         ])]));
 
-        $this->repository->shouldReceive('loadTasks')->with($model)->once()->andReturn($model);
+        $this->scheduleRepository->shouldReceive('loadTasks')->with($model)->once()->andReturn($model);
 
-        $this->repository->shouldReceive('update')->with($model->id, [
+        $this->scheduleRepository->shouldReceive('update')->with($model->id, [
             'is_processing' => true,
             'next_run_at' => Chronos::now()->addSeconds(15),
         ]);
 
-        $this->runnerService->shouldReceive('handle')->with($task)->once()->andReturnNull();
+        $this->taskRepository->shouldReceive('update')->with($task->id, ['is_queued' => true])->once();
+
+        $this->dispatcher->shouldReceive('dispatch')->with(m::on(function ($class) use ($model, $task) {
+            $this->assertInstanceOf(RunTaskJob::class, $class);
+            $this->assertSame($task->time_offset, $class->delay);
+            $this->assertSame($task->id, $class->task);
+            $this->assertSame($model->id, $class->schedule);
+
+            return true;
+        }))->once();
 
-        $this->service->setRunTimeOverride(Chronos::now()->addSeconds(15))->handle($model);
+        $this->getService()->setRunTimeOverride(Chronos::now()->addSeconds(15))->handle($model);
         $this->assertTrue(true);
     }
+
+    private function getService(): ProcessScheduleService
+    {
+        return new ProcessScheduleService($this->dispatcher, $this->scheduleRepository, $this->taskRepository);
+    }
 }