Skip to content

Commit e3178ba

Browse files
committed
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.
1 parent 6066fa4 commit e3178ba

File tree

7 files changed

+58
-25
lines changed

7 files changed

+58
-25
lines changed

app/Console/Commands/Maintenance/PruneOrphanedBackupsCommand.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ class PruneOrphanedBackupsCommand extends Command
1717
/**
1818
* @var string
1919
*/
20-
protected $description = 'Removes all backups that have existed for more than "n" minutes which are not marked as completed.';
20+
protected $description = 'Marks all backups that have not completed in the last "n" minutes as being failed.';
2121

2222
/**
2323
* @param \Pterodactyl\Repositories\Eloquent\BackupRepository $repository
2424
*/
2525
public function handle(BackupRepository $repository)
2626
{
2727
$since = $this->option('since-minutes');
28-
if (!is_digit($since)) {
28+
if (! is_digit($since)) {
2929
throw new InvalidArgumentException('The --since-minutes option must be a valid numeric digit.');
3030
}
3131

@@ -34,14 +34,18 @@ public function handle(BackupRepository $repository)
3434
->whereDate('created_at', '<=', CarbonImmutable::now()->subMinutes($since));
3535

3636
$count = $query->count();
37-
if (!$count) {
38-
$this->info('There are no orphaned backups to be removed.');
37+
if (! $count) {
38+
$this->info('There are no orphaned backups to be marked as failed.');
3939

4040
return;
4141
}
4242

43-
$this->warn("Deleting {$count} backups that have not been marked as completed in the last {$since} minutes.");
43+
$this->warn("Marking {$count} backups that have not been marked as completed in the last {$since} minutes as failed.");
4444

45-
$query->delete();
45+
$query->update([
46+
'is_successful' => false,
47+
'completed_at' => CarbonImmutable::now(),
48+
'updated_at' => CarbonImmutable::now(),
49+
]);
4650
}
4751
}

app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Pterodactyl\Http\Controllers\Api\Remote\Backups;
44

55
use Carbon\Carbon;
6+
use Carbon\CarbonImmutable;
67
use Illuminate\Http\JsonResponse;
78
use Pterodactyl\Http\Controllers\Controller;
89
use Pterodactyl\Repositories\Eloquent\BackupRepository;
@@ -31,25 +32,16 @@ public function __construct(BackupRepository $repository)
3132
* @param \Pterodactyl\Http\Requests\Api\Remote\ReportBackupCompleteRequest $request
3233
* @param string $backup
3334
* @return \Illuminate\Http\JsonResponse
34-
*
35-
* @throws \Pterodactyl\Exceptions\Model\DataValidationException
36-
* @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
3735
*/
3836
public function __invoke(ReportBackupCompleteRequest $request, string $backup)
3937
{
40-
/** @var \Pterodactyl\Models\Backup $backup */
41-
$backup = $this->repository->findFirstWhere([['uuid', '=', $backup]]);
42-
43-
if ($request->input('successful')) {
44-
$this->repository->update($backup->id, [
45-
'sha256_hash' => $request->input('checksum'),
46-
'bytes' => $request->input('size'),
47-
'completed_at' => Carbon::now(),
48-
], true, true);
49-
} else {
50-
$this->repository->delete($backup->id);
51-
}
38+
$this->repository->updateWhere([['uuid', '=', $backup]], [
39+
'is_successful' => $request->input('successful') ? true : false,
40+
'sha256_hash' => $request->input('checksum'),
41+
'bytes' => $request->input('size'),
42+
'completed_at' => CarbonImmutable::now(),
43+
]);
5244

53-
return JsonResponse::create([], JsonResponse::HTTP_NO_CONTENT);
45+
return new JsonResponse([], JsonResponse::HTTP_NO_CONTENT);
5446
}
5547
}

app/Models/Backup.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* @property int $id
99
* @property int $server_id
1010
* @property int $uuid
11+
* @property bool $is_successful
1112
* @property string $name
1213
* @property string[] $ignored_files
1314
* @property string $disk
@@ -44,6 +45,7 @@ class Backup extends Model
4445
*/
4546
protected $casts = [
4647
'id' => 'int',
48+
'is_successful' => 'bool',
4749
'bytes' => 'int',
4850
'ignored_files' => 'array',
4951
];
@@ -59,6 +61,7 @@ class Backup extends Model
5961
* @var array
6062
*/
6163
protected $attributes = [
64+
'is_successful' => true,
6265
'sha256_hash' => null,
6366
'bytes' => 0,
6467
];
@@ -69,6 +72,7 @@ class Backup extends Model
6972
public static $validationRules = [
7073
'server_id' => 'bail|required|numeric|exists:servers,id',
7174
'uuid' => 'required|uuid',
75+
'is_successful' => 'boolean',
7276
'name' => 'required|string',
7377
'ignored_files' => 'array',
7478
'disk' => 'required|string',

app/Repositories/Eloquent/BackupRepository.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public function getBackupsGeneratedDuringTimespan(int $server, int $minutes = 10
2727
return $this->getBuilder()
2828
->withTrashed()
2929
->where('server_id', $server)
30+
->where('is_successful', true)
3031
->where('created_at', '>=', Carbon::now()->subMinutes($minutes)->toDateTimeString())
3132
->get()
3233
->toBase();

app/Services/Backups/InitiateBackupService.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace Pterodactyl\Services\Backups;
44

5-
use Carbon\Carbon;
65
use Ramsey\Uuid\Uuid;
76
use Carbon\CarbonImmutable;
87
use Webmozart\Assert\Assert;
@@ -101,14 +100,14 @@ public function setIgnoredFiles(?array $ignored)
101100
public function handle(Server $server, string $name = null): Backup
102101
{
103102
// Do not allow the user to continue if this server is already at its limit.
104-
if (! $server->backup_limit || $server->backups()->count() >= $server->backup_limit) {
103+
if (! $server->backup_limit || $server->backups()->where('is_successful', true)->count() >= $server->backup_limit) {
105104
throw new TooManyBackupsException($server->backup_limit);
106105
}
107106

108107
$previous = $this->repository->getBackupsGeneratedDuringTimespan($server->id, 10);
109108
if ($previous->count() >= 2) {
110109
throw new TooManyRequestsHttpException(
111-
Carbon::now()->diffInSeconds($previous->last()->created_at->addMinutes(10)),
110+
CarbonImmutable::now()->diffInSeconds($previous->last()->created_at->addMinutes(10)),
112111
'Only two backups may be generated within a 10 minute span of time.'
113112
);
114113
}

app/Transformers/Api/Client/BackupTransformer.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public function transform(Backup $backup)
2222
{
2323
return [
2424
'uuid' => $backup->uuid,
25+
'is_successful' => $backup->is_successful,
2526
'name' => $backup->name,
2627
'ignored_files' => $backup->ignored_files,
2728
'sha256_hash' => $backup->sha256_hash,
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
use Illuminate\Database\Migrations\Migration;
4+
use Illuminate\Database\Schema\Blueprint;
5+
use Illuminate\Support\Facades\Schema;
6+
7+
class AddBackupStateColumnToBackups extends Migration
8+
{
9+
/**
10+
* Run the migrations.
11+
*
12+
* @return void
13+
*/
14+
public function up()
15+
{
16+
Schema::table('backups', function (Blueprint $table) {
17+
$table->boolean('is_successful')->after('uuid')->default(true);
18+
});
19+
}
20+
21+
/**
22+
* Reverse the migrations.
23+
*
24+
* @return void
25+
*/
26+
public function down()
27+
{
28+
Schema::table('backups', function (Blueprint $table) {
29+
$table->dropColumn('is_successful');
30+
});
31+
}
32+
}

0 commit comments

Comments
 (0)