From 2b14e46eec4a71d3c67380f15587f2e5dc8f2f41 Mon Sep 17 00:00:00 2001 From: Matthew Penner Date: Tue, 24 Jan 2023 15:57:24 -0700 Subject: [PATCH] api: fix `sequence_id` being ignored in server task API Closes #4434 --- .../Client/Servers/ScheduleTaskController.php | 76 ++++++++++++++----- .../Servers/Schedules/StoreTaskRequest.php | 1 + 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/app/Http/Controllers/Api/Client/Servers/ScheduleTaskController.php b/app/Http/Controllers/Api/Client/Servers/ScheduleTaskController.php index addca2876..53f5f7dc8 100644 --- a/app/Http/Controllers/Api/Client/Servers/ScheduleTaskController.php +++ b/app/Http/Controllers/Api/Client/Servers/ScheduleTaskController.php @@ -9,6 +9,7 @@ use Pterodactyl\Models\Schedule; use Illuminate\Http\JsonResponse; use Pterodactyl\Facades\Activity; use Pterodactyl\Models\Permission; +use Illuminate\Database\ConnectionInterface; use Pterodactyl\Repositories\Eloquent\TaskRepository; use Pterodactyl\Exceptions\Http\HttpForbiddenException; use Pterodactyl\Transformers\Api\Client\TaskTransformer; @@ -23,8 +24,10 @@ class ScheduleTaskController extends ClientApiController /** * ScheduleTaskController constructor. */ - public function __construct(private TaskRepository $repository) - { + public function __construct( + private ConnectionInterface $connection, + private TaskRepository $repository + ) { parent::__construct(); } @@ -49,14 +52,30 @@ class ScheduleTaskController extends ClientApiController $lastTask = $schedule->tasks()->orderByDesc('sequence_id')->first(); /** @var \Pterodactyl\Models\Task $task */ - $task = $this->repository->create([ - 'schedule_id' => $schedule->id, - 'sequence_id' => ($lastTask->sequence_id ?? 0) + 1, - 'action' => $request->input('action'), - 'payload' => $request->input('payload') ?? '', - 'time_offset' => $request->input('time_offset'), - 'continue_on_failure' => (bool) $request->input('continue_on_failure'), - ]); + $task = $this->connection->transaction(function () use ($request, $schedule, $lastTask) { + $sequenceId = ($lastTask->sequence_id ?? 0) + 1; + $requestSequenceId = $request->integer('sequence_id', $sequenceId); + + // If the sequence id from the request is greater than or equal to the next available + // sequence id, we don't need to do anything special. Otherwise, we need to update + // the sequence id of all tasks that are greater than or equal to the request sequence + // id to be one greater than the current value. + if ($requestSequenceId < $sequenceId) { + $schedule->tasks() + ->where('sequence_id', '>=', $requestSequenceId) + ->increment('sequence_id'); + $sequenceId = $requestSequenceId; + } + + return $this->repository->create([ + 'schedule_id' => $schedule->id, + 'sequence_id' => $sequenceId, + 'action' => $request->input('action'), + 'payload' => $request->input('payload') ?? '', + 'time_offset' => $request->input('time_offset'), + 'continue_on_failure' => $request->boolean('continue_on_failure'), + ]); + }); Activity::event('server:task.create') ->subject($schedule, $task) @@ -84,12 +103,30 @@ class ScheduleTaskController extends ClientApiController throw new HttpForbiddenException("A backup task cannot be created when the server's backup limit is set to 0."); } - $this->repository->update($task->id, [ - 'action' => $request->input('action'), - 'payload' => $request->input('payload') ?? '', - 'time_offset' => $request->input('time_offset'), - 'continue_on_failure' => (bool) $request->input('continue_on_failure'), - ]); + $this->connection->transaction(function () use ($request, $schedule, $task) { + $sequenceId = $request->integer('sequence_id', $task->sequence_id); + + // Shift all other tasks in the schedule up or down to make room for the new task. + if ($sequenceId < $task->sequence_id) { + $schedule->tasks() + ->where('sequence_id', '>=', $sequenceId) + ->where('sequence_id', '<', $task->sequence_id) + ->increment('sequence_id'); + } elseif ($sequenceId > $task->sequence_id) { + $schedule->tasks() + ->where('sequence_id', '>', $task->sequence_id) + ->where('sequence_id', '<=', $sequenceId) + ->decrement('sequence_id'); + } + + $this->repository->update($task->id, [ + 'sequence_id' => $sequenceId, + 'action' => $request->input('action'), + 'payload' => $request->input('payload') ?? '', + 'time_offset' => $request->input('time_offset'), + 'continue_on_failure' => $request->boolean('continue_on_failure'), + ]); + }); Activity::event('server:task.update') ->subject($schedule, $task) @@ -117,10 +154,9 @@ class ScheduleTaskController extends ClientApiController throw new HttpForbiddenException('You do not have permission to perform this action.'); } - $schedule->tasks()->where('sequence_id', '>', $task->sequence_id)->update([ - 'sequence_id' => $schedule->tasks()->getConnection()->raw('(sequence_id - 1)'), - ]); - + $schedule->tasks() + ->where('sequence_id', '>', $task->sequence_id) + ->decrement('sequence_id'); $task->delete(); Activity::event('server:task.delete')->subject($schedule, $task)->property('name', $schedule->name)->log(); diff --git a/app/Http/Requests/Api/Client/Servers/Schedules/StoreTaskRequest.php b/app/Http/Requests/Api/Client/Servers/Schedules/StoreTaskRequest.php index 811b87dfb..5ceb7c8e0 100644 --- a/app/Http/Requests/Api/Client/Servers/Schedules/StoreTaskRequest.php +++ b/app/Http/Requests/Api/Client/Servers/Schedules/StoreTaskRequest.php @@ -23,6 +23,7 @@ class StoreTaskRequest extends ViewScheduleRequest 'payload' => 'required_unless:action,backup|string|nullable', 'time_offset' => 'required|numeric|min:0|max:900', 'sequence_id' => 'sometimes|required|numeric|min:1', + 'continue_on_failure' => 'sometimes|required|boolean', ]; } }