From ae61299068e2adf95f5daee0877e2fe0d4eff655 Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Thu, 27 Jun 2024 13:10:22 +0200 Subject: [PATCH 1/4] Fix issue where deleted models' files were not removed Remove attribute to enable file deletion. --- src/Jobs/DeleteStorageRequestFile.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Jobs/DeleteStorageRequestFile.php b/src/Jobs/DeleteStorageRequestFile.php index f87cc7b..c1a42bc 100644 --- a/src/Jobs/DeleteStorageRequestFile.php +++ b/src/Jobs/DeleteStorageRequestFile.php @@ -47,8 +47,6 @@ class DeleteStorageRequestFile extends Job implements ShouldQueue * **/ public $oldRetryCount; - public $deleteWhenMissingModels = true; - /** * Create a new job instance. * @@ -76,7 +74,7 @@ public function __construct(StorageRequestFile $file) public function handle() { // Do not delete files when delete-request is outdated - if($this->oldRetryCount != $this->file->refresh()->retry_count) { + if($this->file->exists() && $this->oldRetryCount != $this->file->refresh()->retry_count) { return; } @@ -110,6 +108,8 @@ public function handle() } } - $this->file->delete(); + if($this->file->exists()) { + $this->file->delete(); + } } } From b8b616d093ed4f4066736ff83f9632d811fe11e3 Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Thu, 27 Jun 2024 13:26:03 +0200 Subject: [PATCH 2/4] Add test --- tests/Jobs/DeleteStorageRequestFileTest.php | 25 +++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/Jobs/DeleteStorageRequestFileTest.php b/tests/Jobs/DeleteStorageRequestFileTest.php index 5809fe4..84b1023 100644 --- a/tests/Jobs/DeleteStorageRequestFileTest.php +++ b/tests/Jobs/DeleteStorageRequestFileTest.php @@ -119,4 +119,29 @@ public function testOutdatedDeleteJob(){ $this->assertTrue($disk->exists("request-{$file->storage_request_id}/a.jpg.2")); $this->assertModelExists($file); } + + public function testMissingModel(){ + config(['user_storage.storage_disk' => 'test']); + $disk = Storage::fake('test'); + + $request = StorageRequest::factory()->create([ + 'expires_at' => now()->subWeeks(2), + ]); + + $file = StorageRequestFile::factory()->create([ + 'path' => 'image.png', + 'storage_request_id' => $request->id, + 'retry_count' => 1, + ]); + + $disk->put('user-1/image.png',''); + + $job = new DeleteStorageRequestFile($file); + $request->delete(); // Simulate deleted models + $job->handle(); + + $this->assertFalse($disk->exists("user-{$request->user_id}/a.jpg")); + $this->assertModelMissing($request); + $this->assertModelMissing($file); + } } From 451794681d715968788ff2cc74bbdd530f66de5f Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Thu, 27 Jun 2024 13:39:10 +0200 Subject: [PATCH 3/4] Fix wrong file name in test --- tests/Jobs/DeleteStorageRequestFileTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Jobs/DeleteStorageRequestFileTest.php b/tests/Jobs/DeleteStorageRequestFileTest.php index 84b1023..24973e9 100644 --- a/tests/Jobs/DeleteStorageRequestFileTest.php +++ b/tests/Jobs/DeleteStorageRequestFileTest.php @@ -140,7 +140,7 @@ public function testMissingModel(){ $request->delete(); // Simulate deleted models $job->handle(); - $this->assertFalse($disk->exists("user-{$request->user_id}/a.jpg")); + $this->assertFalse($disk->exists("user-{$request->user_id}/image.png")); $this->assertModelMissing($request); $this->assertModelMissing($file); } From 8d6fd3aa18f8f1e18dbb8dedd8819209c39463ba Mon Sep 17 00:00:00 2001 From: Martin Zurowietz Date: Thu, 27 Jun 2024 14:21:42 +0200 Subject: [PATCH 4/4] Add scalar path to delete file job --- src/Jobs/DeleteStorageRequestFile.php | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Jobs/DeleteStorageRequestFile.php b/src/Jobs/DeleteStorageRequestFile.php index c1a42bc..659af02 100644 --- a/src/Jobs/DeleteStorageRequestFile.php +++ b/src/Jobs/DeleteStorageRequestFile.php @@ -16,10 +16,16 @@ class DeleteStorageRequestFile extends Job implements ShouldQueue /** * File that should be deleted - * - * **/ + */ public $file; + /** + * Path of the file to be deleted. + * + * @var string + */ + public $path; + /** * Whether the request of the file was pending or not. * @@ -55,6 +61,7 @@ class DeleteStorageRequestFile extends Job implements ShouldQueue public function __construct(StorageRequestFile $file) { $this->file = $file; + $this->path = $file->path; $this->oldRetryCount = $file->retry_count; $request = $file->request; $this->pending = is_null($request->expires_at); @@ -73,8 +80,10 @@ public function __construct(StorageRequestFile $file) */ public function handle() { + $fileExists = $this->file && $this->file->exists(); + // Do not delete files when delete-request is outdated - if($this->file->exists() && $this->oldRetryCount != $this->file->refresh()->retry_count) { + if ($fileExists && $this->oldRetryCount != $this->file->refresh()->retry_count) { return; } @@ -84,7 +93,7 @@ public function handle() $disk = Storage::disk(config('user_storage.storage_disk')); } - $path = "{$this->prefix}/{$this->file->path}"; + $path = "{$this->prefix}/{$this->path}"; if ($this->chunks) { $paths = array_map(function ($chunk) use ($path) { @@ -97,7 +106,7 @@ public function handle() } if (!$success) { - throw new Exception("Could not delete file '{$this->file->path}' for storage request with prefix '{$this->prefix}'."); + throw new Exception("Could not delete file '{$this->path}' for storage request with prefix '{$this->prefix}'."); } if ($success && count($disk->allFiles($this->prefix)) === 0) { @@ -108,7 +117,7 @@ public function handle() } } - if($this->file->exists()) { + if ($fileExists) { $this->file->delete(); } }