From bf72fd5d8823c788d5f1f8eb18898f9839ed6546 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sun, 19 Jul 2026 23:20:46 +0200 Subject: [PATCH] refactor(backups): centralize storage deletion guard --- .../Api/ApplicationsController.php | 8 +------ .../Controllers/Api/DatabasesController.php | 8 +------ .../Controllers/Api/ServicesController.php | 8 +------ app/Models/LocalFileVolume.php | 7 +++++++ app/Models/LocalPersistentVolume.php | 7 +++++++ tests/Feature/VolumeBackupTest.php | 21 +++++++++++++++++++ 6 files changed, 38 insertions(+), 21 deletions(-) diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index 1305ab795..c2592d7f9 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -4910,13 +4910,7 @@ public function delete_storage(Request $request): JsonResponse ], 422); } - if ($storage->scheduledBackups()->exists()) { - return response()->json([ - 'message' => $storage instanceof LocalFileVolume - ? 'Delete this directory backup schedule and its archives before deleting the directory.' - : 'Delete this volume backup schedule and its archives before deleting the volume.', - ], 422); - } + $storage->abortIfScheduledBackupsExist(); if ($storage instanceof LocalFileVolume) { $storage->deleteStorageOnServer(); diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index 00ce233cc..41af38a72 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -4484,13 +4484,7 @@ public function delete_storage(Request $request): JsonResponse ], 422); } - if ($storage->scheduledBackups()->exists()) { - return response()->json([ - 'message' => $storage instanceof LocalFileVolume - ? 'Delete this directory backup schedule and its archives before deleting the directory.' - : 'Delete this volume backup schedule and its archives before deleting the volume.', - ], 422); - } + $storage->abortIfScheduledBackupsExist(); if ($storage instanceof LocalFileVolume) { $storage->deleteStorageOnServer(); diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index 0a1d156a7..9945dfcf9 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -2911,13 +2911,7 @@ public function delete_storage(Request $request): JsonResponse ], 422); } - if ($storage->scheduledBackups()->exists()) { - return response()->json([ - 'message' => $storage instanceof LocalFileVolume - ? 'Delete this directory backup schedule and its archives before deleting the directory.' - : 'Delete this volume backup schedule and its archives before deleting the volume.', - ], 422); - } + $storage->abortIfScheduledBackupsExist(); if ($storage instanceof LocalFileVolume) { $storage->deleteStorageOnServer(); diff --git a/app/Models/LocalFileVolume.php b/app/Models/LocalFileVolume.php index a03ec486b..86873d1a1 100644 --- a/app/Models/LocalFileVolume.php +++ b/app/Models/LocalFileVolume.php @@ -96,6 +96,13 @@ public function scheduledBackups(): MorphMany return $this->morphMany(ScheduledVolumeBackup::class, 'backupable'); } + public function abortIfScheduledBackupsExist(): void + { + if ($this->scheduledBackups()->exists()) { + abort(422, 'Delete this directory backup schedule and its archives before deleting the directory.'); + } + } + public function loadStorageOnServer() { if ($this->is_host_file) { diff --git a/app/Models/LocalPersistentVolume.php b/app/Models/LocalPersistentVolume.php index 72bb3c38d..6b4e0fe5b 100644 --- a/app/Models/LocalPersistentVolume.php +++ b/app/Models/LocalPersistentVolume.php @@ -56,6 +56,13 @@ public function scheduledBackups(): MorphMany return $this->morphMany(ScheduledVolumeBackup::class, 'backupable'); } + public function abortIfScheduledBackupsExist(): void + { + if ($this->scheduledBackups()->exists()) { + abort(422, 'Delete this volume backup schedule and its archives before deleting the volume.'); + } + } + protected function customizeName($value) { return str($value)->trim()->value; diff --git a/tests/Feature/VolumeBackupTest.php b/tests/Feature/VolumeBackupTest.php index 9b650b942..8a8058ff4 100644 --- a/tests/Feature/VolumeBackupTest.php +++ b/tests/Feature/VolumeBackupTest.php @@ -37,6 +37,7 @@ use Illuminate\Support\Facades\Schema; use Illuminate\Support\Facades\Storage; use Livewire\Livewire; +use Symfony\Component\HttpKernel\Exception\HttpException; uses(RefreshDatabase::class); @@ -1456,6 +1457,26 @@ function signInForVolumeBackups($testCase, Team $team): User expect($volume->fresh())->not->toBeNull(); }); +it('aborts storage deletion when scheduled backups exist', function () { + $team = Team::factory()->create(); + [$application, $volume] = createVolumeBackupApplication($team); + $directory = createApplicationBackupDirectory($application); + + $volume->scheduledBackups()->create([ + 'team_id' => $team->id, + 'frequency' => 'daily', + ]); + $directory->scheduledBackups()->create([ + 'team_id' => $team->id, + 'frequency' => 'daily', + ]); + + expect(fn () => $volume->abortIfScheduledBackupsExist()) + ->toThrow(HttpException::class, 'Delete this volume backup schedule and its archives before deleting the volume.') + ->and(fn () => $directory->abortIfScheduledBackupsExist()) + ->toThrow(HttpException::class, 'Delete this directory backup schedule and its archives before deleting the directory.'); +}); + it('deletes disabled volume backup archives before deleting their application', function () { Process::fake(); Queue::fake();