Fix delete job again

The file model in the job was populated and exists() returned true even if the model no longer existed in the DB. Now only the ID is stored and the model is newly fetched when the job runs. The model will now be null when it no longer exists.
biigle · Jul 1, 2024 · 669ca61 · 669ca61
1 parent 6cedad0
commit 669ca61
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 17 deletions.
diff --git a/src/Jobs/DeleteStorageRequestFile.php b/src/Jobs/DeleteStorageRequestFile.php
@@ -15,9 +15,9 @@ class DeleteStorageRequestFile extends Job implements ShouldQueue
     use InteractsWithQueue, Batchable;
 
     /**
-     * File that should be deleted
+     * ID of the file that should be deleted
      */
-    public $file;
+    public $fileId;
 
     /**
      * Path of the file to be deleted.
@@ -60,7 +60,7 @@ class DeleteStorageRequestFile extends Job implements ShouldQueue
      */
     public function __construct(StorageRequestFile $file)
     {
-        $this->file = $file;
+        $this->fileId = $file->id;
         $this->path = $file->path;
         $this->oldRetryCount = $file->retry_count;
         $request = $file->request;
@@ -80,10 +80,10 @@ public function __construct(StorageRequestFile $file)
      */
     public function handle()
     {
-        $fileExists = $this->file && $this->file->exists();
+        $file = StorageRequestFile::find($this->fileId);
 
         // Do not delete files when delete-request is outdated
-        if ($fileExists && $this->oldRetryCount != $this->file->refresh()->retry_count) {
+        if ($file && $this->oldRetryCount != $file->retry_count) {
             return;
         }
 
@@ -117,8 +117,8 @@ public function handle()
             }
         }
 
-        if ($fileExists) {
-            $this->file->delete();
+        if ($file) {
+            $file->delete();
         }
     }
 }
diff --git a/tests/Console/Commands/PruneExpiredStorageRequestsTest.php b/tests/Console/Commands/PruneExpiredStorageRequestsTest.php
@@ -41,7 +41,7 @@ public function testHandle()
 
         Bus::assertBatched(function (PendingBatch $batch) use ($file1) {
             $this->assertEquals(1, $batch->jobs->count());
-            $this->assertEquals($file1->path, $batch->jobs->first()->file->path);
+            $this->assertEquals($file1->id, $batch->jobs->first()->fileId);
             return true;
         });
 

diff --git a/tests/Http/Controllers/Api/StorageRequestControllerTest.php b/tests/Http/Controllers/Api/StorageRequestControllerTest.php
@@ -352,7 +352,7 @@ public function testDestroy()
 
         Bus::assertBatched(function (PendingBatch $batch) use ($file) {
             $this->assertEquals(1, $batch->jobs->count());
-            $this->assertEquals($file->path, $batch->jobs->first()->file->path);
+            $this->assertEquals($file->id, $batch->jobs->first()->fileId);
             return true;
         });
         $this->assertNull($request->fresh());

diff --git a/tests/Http/Controllers/Api/StorageRequestDirectoryControllerTest.php b/tests/Http/Controllers/Api/StorageRequestDirectoryControllerTest.php
@@ -48,8 +48,8 @@ public function testDestory()
         $this->assertNull($file1->fresh());
         $this->assertNotNull($file2->fresh());
 
-        Queue::assertPushed(function (DeleteStorageRequestFile $job) {
-            $this->assertSame('a/a.jpg', $job->file->path);
+        Queue::assertPushed(function (DeleteStorageRequestFile $job) use ($file1) {
+            $this->assertSame($file1->id, $job->fileId);
 
             return true;
         });

diff --git a/tests/Http/Controllers/Api/StorageRequestFileControllerTest.php b/tests/Http/Controllers/Api/StorageRequestFileControllerTest.php
@@ -367,7 +367,7 @@ public function testStoreChunkTooLargeQuota()
             ->assertStatus(422);
 
         Bus::assertDispatched(function (DeleteStorageRequestFile $job) {
-            $this->assertSame('test.jpg', $job->file->path);
+            $this->assertSame('test.jpg', $job->path);
             $this->assertSame([0], $job->chunks);
 
             return true;
@@ -420,7 +420,7 @@ public function testStoreChunkTooLargeFile()
 
 
         Bus::assertDispatched(function (DeleteStorageRequestFile $job) {
-            $this->assertSame('test.jpg', $job->file->path);
+            $this->assertSame('test.jpg', $job->path);
             $this->assertSame([0], $job->chunks);
 
             return true;
@@ -794,12 +794,11 @@ public function testDestory()
 
         $this->assertNotNull($file->fresh());
 
-        Bus::assertDispatched(function (DeleteStorageRequestFile $job) {
-            $this->assertSame('a.jpg', $job->file->path);
+        Bus::assertDispatched(function (DeleteStorageRequestFile $job) use ($file) {
+            $this->assertSame($file->id, $job->fileId);
 
             return true;
         });
-
     }
 
     public function testDestoryLastFile()

diff --git a/tests/StorageRequestTest.php b/tests/StorageRequestTest.php
@@ -50,7 +50,7 @@ public function testDeleteApprovedFiles()
         $this->model->delete();
         Bus::assertBatched(function (PendingBatch $batch) use ($file) {
             $this->assertEquals(1, $batch->jobs->count());
-            $this->assertEquals($file->path, $batch->jobs->first()->file->path);
+            $this->assertEquals($file->id, $batch->jobs->first()->fileId);
             return true;
         });
     }