From e3178ba6f059044883770e36464482a575c04aab Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Thu, 20 Aug 2020 21:07:53 -0700 Subject: [PATCH] backend: support is_successful state for backups rather than deleting it when failing This allows the UI to correctly show failed backups to the user and require them to manually delete those backups, rather than them mysteriously disappearing. We can also hook into this later to send a notification to the user when the backup fails. --- .../PruneOrphanedBackupsCommand.php | 16 ++++++---- .../Remote/Backups/BackupStatusController.php | 24 +++++--------- app/Models/Backup.php | 4 +++ .../Eloquent/BackupRepository.php | 1 + .../Backups/InitiateBackupService.php | 5 ++- .../Api/Client/BackupTransformer.php | 1 + ...533_add_backup_state_column_to_backups.php | 32 +++++++++++++++++++ 7 files changed, 58 insertions(+), 25 deletions(-) create mode 100644 database/migrations/2020_08_20_205533_add_backup_state_column_to_backups.php diff --git a/app/Console/Commands/Maintenance/PruneOrphanedBackupsCommand.php b/app/Console/Commands/Maintenance/PruneOrphanedBackupsCommand.php index d1dbb033..af4590d4 100644 --- a/app/Console/Commands/Maintenance/PruneOrphanedBackupsCommand.php +++ b/app/Console/Commands/Maintenance/PruneOrphanedBackupsCommand.php @@ -17,7 +17,7 @@ class PruneOrphanedBackupsCommand extends Command /** * @var string */ - protected $description = 'Removes all backups that have existed for more than "n" minutes which are not marked as completed.'; + protected $description = 'Marks all backups that have not completed in the last "n" minutes as being failed.'; /** * @param \Pterodactyl\Repositories\Eloquent\BackupRepository $repository @@ -25,7 +25,7 @@ class PruneOrphanedBackupsCommand extends Command public function handle(BackupRepository $repository) { $since = $this->option('since-minutes'); - if (!is_digit($since)) { + if (! is_digit($since)) { throw new InvalidArgumentException('The --since-minutes option must be a valid numeric digit.'); } @@ -34,14 +34,18 @@ class PruneOrphanedBackupsCommand extends Command ->whereDate('created_at', '<=', CarbonImmutable::now()->subMinutes($since)); $count = $query->count(); - if (!$count) { - $this->info('There are no orphaned backups to be removed.'); + if (! $count) { + $this->info('There are no orphaned backups to be marked as failed.'); return; } - $this->warn("Deleting {$count} backups that have not been marked as completed in the last {$since} minutes."); + $this->warn("Marking {$count} backups that have not been marked as completed in the last {$since} minutes as failed."); - $query->delete(); + $query->update([ + 'is_successful' => false, + 'completed_at' => CarbonImmutable::now(), + 'updated_at' => CarbonImmutable::now(), + ]); } } diff --git a/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php b/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php index bb7d94c1..be971d60 100644 --- a/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php +++ b/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php @@ -3,6 +3,7 @@ namespace Pterodactyl\Http\Controllers\Api\Remote\Backups; use Carbon\Carbon; +use Carbon\CarbonImmutable; use Illuminate\Http\JsonResponse; use Pterodactyl\Http\Controllers\Controller; use Pterodactyl\Repositories\Eloquent\BackupRepository; @@ -31,25 +32,16 @@ class BackupStatusController extends Controller * @param \Pterodactyl\Http\Requests\Api\Remote\ReportBackupCompleteRequest $request * @param string $backup * @return \Illuminate\Http\JsonResponse - * - * @throws \Pterodactyl\Exceptions\Model\DataValidationException - * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException */ public function __invoke(ReportBackupCompleteRequest $request, string $backup) { - /** @var \Pterodactyl\Models\Backup $backup */ - $backup = $this->repository->findFirstWhere([['uuid', '=', $backup]]); + $this->repository->updateWhere([['uuid', '=', $backup]], [ + 'is_successful' => $request->input('successful') ? true : false, + 'sha256_hash' => $request->input('checksum'), + 'bytes' => $request->input('size'), + 'completed_at' => CarbonImmutable::now(), + ]); - if ($request->input('successful')) { - $this->repository->update($backup->id, [ - 'sha256_hash' => $request->input('checksum'), - 'bytes' => $request->input('size'), - 'completed_at' => Carbon::now(), - ], true, true); - } else { - $this->repository->delete($backup->id); - } - - return JsonResponse::create([], JsonResponse::HTTP_NO_CONTENT); + return new JsonResponse([], JsonResponse::HTTP_NO_CONTENT); } } diff --git a/app/Models/Backup.php b/app/Models/Backup.php index 14cb4dde..e164b1ae 100644 --- a/app/Models/Backup.php +++ b/app/Models/Backup.php @@ -8,6 +8,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @property int $id * @property int $server_id * @property int $uuid + * @property bool $is_successful * @property string $name * @property string[] $ignored_files * @property string $disk @@ -44,6 +45,7 @@ class Backup extends Model */ protected $casts = [ 'id' => 'int', + 'is_successful' => 'bool', 'bytes' => 'int', 'ignored_files' => 'array', ]; @@ -59,6 +61,7 @@ class Backup extends Model * @var array */ protected $attributes = [ + 'is_successful' => true, 'sha256_hash' => null, 'bytes' => 0, ]; @@ -69,6 +72,7 @@ class Backup extends Model public static $validationRules = [ 'server_id' => 'bail|required|numeric|exists:servers,id', 'uuid' => 'required|uuid', + 'is_successful' => 'boolean', 'name' => 'required|string', 'ignored_files' => 'array', 'disk' => 'required|string', diff --git a/app/Repositories/Eloquent/BackupRepository.php b/app/Repositories/Eloquent/BackupRepository.php index 4afdd4bd..adbbd9c9 100644 --- a/app/Repositories/Eloquent/BackupRepository.php +++ b/app/Repositories/Eloquent/BackupRepository.php @@ -27,6 +27,7 @@ class BackupRepository extends EloquentRepository return $this->getBuilder() ->withTrashed() ->where('server_id', $server) + ->where('is_successful', true) ->where('created_at', '>=', Carbon::now()->subMinutes($minutes)->toDateTimeString()) ->get() ->toBase(); diff --git a/app/Services/Backups/InitiateBackupService.php b/app/Services/Backups/InitiateBackupService.php index da11d922..304386a6 100644 --- a/app/Services/Backups/InitiateBackupService.php +++ b/app/Services/Backups/InitiateBackupService.php @@ -2,7 +2,6 @@ namespace Pterodactyl\Services\Backups; -use Carbon\Carbon; use Ramsey\Uuid\Uuid; use Carbon\CarbonImmutable; use Webmozart\Assert\Assert; @@ -101,14 +100,14 @@ class InitiateBackupService public function handle(Server $server, string $name = null): Backup { // Do not allow the user to continue if this server is already at its limit. - if (! $server->backup_limit || $server->backups()->count() >= $server->backup_limit) { + if (! $server->backup_limit || $server->backups()->where('is_successful', true)->count() >= $server->backup_limit) { throw new TooManyBackupsException($server->backup_limit); } $previous = $this->repository->getBackupsGeneratedDuringTimespan($server->id, 10); if ($previous->count() >= 2) { throw new TooManyRequestsHttpException( - Carbon::now()->diffInSeconds($previous->last()->created_at->addMinutes(10)), + CarbonImmutable::now()->diffInSeconds($previous->last()->created_at->addMinutes(10)), 'Only two backups may be generated within a 10 minute span of time.' ); } diff --git a/app/Transformers/Api/Client/BackupTransformer.php b/app/Transformers/Api/Client/BackupTransformer.php index 53966fc7..15d6b357 100644 --- a/app/Transformers/Api/Client/BackupTransformer.php +++ b/app/Transformers/Api/Client/BackupTransformer.php @@ -22,6 +22,7 @@ class BackupTransformer extends BaseClientTransformer { return [ 'uuid' => $backup->uuid, + 'is_successful' => $backup->is_successful, 'name' => $backup->name, 'ignored_files' => $backup->ignored_files, 'sha256_hash' => $backup->sha256_hash, diff --git a/database/migrations/2020_08_20_205533_add_backup_state_column_to_backups.php b/database/migrations/2020_08_20_205533_add_backup_state_column_to_backups.php new file mode 100644 index 00000000..a0856849 --- /dev/null +++ b/database/migrations/2020_08_20_205533_add_backup_state_column_to_backups.php @@ -0,0 +1,32 @@ +boolean('is_successful')->after('uuid')->default(true); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('backups', function (Blueprint $table) { + $table->dropColumn('is_successful'); + }); + } +}