Upgrade to Laravel 9

src/Jobs/ProcessDelphiJob.php

@@ -10,14 +10,15 @@
 use Exception;
 use File;
 use FileCache;
+use Illuminate\Bus\Batchable;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Queue\InteractsWithQueue;
 use Illuminate\Queue\SerializesModels;
 use Storage;
 
 class ProcessDelphiJob extends BaseJob implements ShouldQueue
 {
-    use InteractsWithQueue, SerializesModels;
+    use Batchable, InteractsWithQueue, SerializesModels;
 
     /**
      * The image to process.
@@ -40,14 +41,6 @@ class ProcessDelphiJob extends BaseJob implements ShouldQueue
      */
     protected $distance;
 
-    /**
-     * Key of the cached count of other jobs that run on the same volume than this one.
-     * The last job should delete the gatherFile.
-     *
-     * @var string
-     */
-    protected $cacheKey;
-
     /**
      * Ignore this job if the image does not exist any more.
      *
@@ -72,13 +65,11 @@ class ProcessDelphiJob extends BaseJob implements ShouldQueue
      *
      * @return void
      */
-    public function __construct($image, $distance, $gatherFile, $cacheKey = null)
+    public function __construct($image, $distance, $gatherFile)
     {
         $this->image = Image::convert($image);
         $this->gatherFile = $gatherFile;
         $this->distance = $distance;
-        // If null, this job assumes it is the only one accessing the gatherFile.
-        $this->cacheKey = $cacheKey;
     }
 
     /**
@@ -115,40 +106,5 @@ public function handle()
 
         $this->image->laserpoints = $output;
         $this->image->save();
-
-        $this->maybeDeleteGatherFile();
-    }
-
-    /**
-     * Handle a job failure.
-     *
-     * @return void
-     */
-    public function failed()
-    {
-        $this->maybeDeleteGatherFile();
-    }
-
-    /**
-     * Handles the deletion of the gatherFile once all "sibling" jobs finished.
-     *
-     * If more than one image is processed during a Delphi LP detection, the jobs use the
-     * cache to track how many of them are still running. They need to track this to
-     * determine when the gatherFile can be deleted. This function updates the count
-     * when a job was finished and deletes the gather file if this is the last job to
-     * finish.
-     */
-    protected function maybeDeleteGatherFile()
-    {
-        if ($this->cacheKey) {
-            // This requires an atomic operation to work correctly. BIIGLE uses the
-            // Redis cache which provides these atomic operations.
-            if (Cache::decrement($this->cacheKey) > 0) {
-                return;
-            }
-            Cache::forget($this->cacheKey);
-        }
-
-        Storage::disk(config('laserpoints.disk'))->delete($this->gatherFile);
     }
 }

src/Jobs/ProcessImageDelphiJob.php

@@ -3,6 +3,8 @@
 namespace Biigle\Modules\Laserpoints\Jobs;
 
 use Biigle\Image;
+use Illuminate\Support\Facades\Bus;
+use Storage;
 
 class ProcessImageDelphiJob extends Job
 {
@@ -42,7 +44,13 @@ public function handle()
 
         $points = $this->getLaserpointsForVolume($this->image->volume_id);
         $gatherFile = $this->gather($points);
-        ProcessDelphiJob::dispatch($this->image, $this->distance, $gatherFile)
-            ->onQueue(config('laserpoints.process_delphi_queue'));
+        $job = new ProcessDelphiJob($this->image, $this->distance, $gatherFile);
+
+        Bus::batch([$job])
+            ->onQueue(config('laserpoints.process_delphi_queue'))
+            ->finally(function () use ($gatherFile) {
+                Storage::disk(config('laserpoints.disk'))->delete($gatherFile);
+            })
+            ->dispatch();
     }
 }

src/Jobs/ProcessVolumeDelphiJob.php

@@ -6,6 +6,8 @@
 use Biigle\Volume;
 use Cache;
 use Illuminate\Queue\SerializesModels;
+use Illuminate\Support\Facades\Bus;
+use Storage;
 
 class ProcessVolumeDelphiJob extends Job
 {
@@ -50,25 +52,30 @@ public function handle()
         $points = $this->getLaserpointsForVolume($this->volume->id);
         $images = Image::whereIn('id', $points->keys())->get();
 
-        foreach ($images as $image) {
-            ProcessManualJob::dispatch($image, $points->get($image->id), $this->distance)
-                ->onQueue(config('laserpoints.process_manual_queue'));
-        }
+        $jobs = $images->map(function ($image) use ($points) {
+            return new ProcessManualJob($image, $points->get($image->id), $this->distance);
+        });
 
+        Bus::batch($jobs)
+            ->onQueue(config('laserpoints.process_manual_queue'))
+            ->dispatch();
 
         $query = $this->volume->images()->whereNotIn('id', $points->keys());
 
         if ($query->exists()) {
-            $imagesToProcess = $query->get();
+            $images = $query->get();
             $gatherFile = $this->gather($points);
 
-            $cacheKey = uniqid('delphi_job_count_');
-            Cache::forever($cacheKey, $imagesToProcess->count());
+            $jobs = $images->map(function ($image) use ($gatherFile) {
+                return new ProcessDelphiJob($image, $this->distance, $gatherFile);
+            });
 
-            foreach ($imagesToProcess as $image) {
-                ProcessDelphiJob::dispatch($image, $this->distance, $gatherFile, $cacheKey)
-                    ->onQueue(config('laserpoints.process_delphi_queue'));
-            }
+            Bus::batch($jobs)
+                ->onQueue(config('laserpoints.process_delphi_queue'))
+                ->finally(function () use ($gatherFile) {
+                    Storage::disk(config('laserpoints.disk'))->delete($gatherFile);
+                })
+                ->dispatch();
         }
     }
 }

tests/Jobs/ProcessDelphiJobTest.php

@@ -64,54 +64,10 @@ public function testHandle()
         $this->assertEquals($expect, $this->image->fresh()->laserpoints);
         // Previously set attrs should not be lost.
         $this->assertEquals(1, $this->image->fresh()->attrs['a']);
-        $this->assertFalse($disk->exists($this->gatherFile));
-    }
-
-    public function testHandleCountDecrease()
-    {
-        $disk = Storage::disk('laserpoints');
-        $disk->put($this->gatherFile, 'test');
-        Cache::forever('test_job_count', 2);
-
-        $mock = Mockery::mock(DelphiApply::class);
-        $mock->shouldReceive('execute')
-            ->once()
-            ->andReturn([]);
-
-        App::singleton(DelphiApply::class, function () use ($mock) {
-            return $mock;
-        });
-
-        with(new ProcessDelphiJob($this->image, 30, $this->gatherFile, 'test_job_count'))->handle();
-
-        $this->assertEquals(1, Cache::get('test_job_count'));
-        $this->assertTrue($disk->exists($this->gatherFile));
-    }
-
-    public function testHandleCountZero()
-    {
-        $disk = Storage::disk('laserpoints');
-        $disk->put($this->gatherFile, 'test');
-        Cache::forever('test_job_count', 1);
-
-        $mock = Mockery::mock(DelphiApply::class);
-        $mock->shouldReceive('execute')
-            ->once()
-            ->andReturn([]);
-
-        App::singleton(DelphiApply::class, function () use ($mock) {
-            return $mock;
-        });
-
-        with(new ProcessDelphiJob($this->image, 30, $this->gatherFile, 'test_job_count'))->handle();
-        $this->assertFalse(Cache::has('test_job_count'));
-        $this->assertFalse($disk->exists($this->gatherFile));
     }
 
     public function testHandleGracefulError()
     {
-        $disk = Storage::disk('laserpoints');
-        $disk->put($this->gatherFile, 'test');
         $mock = Mockery::mock(DelphiApply::class);
         $mock->shouldReceive('execute')
             ->once()
@@ -133,13 +89,10 @@ public function testHandleGracefulError()
         ];
 
         $this->assertEquals($expect, $this->image->fresh()->laserpoints);
-        $this->assertFalse($disk->exists($this->gatherFile));
     }
 
     public function testHandleFatalError()
     {
-        $disk = Storage::disk('laserpoints');
-        $disk->put($this->gatherFile, 'test');
         // previous laserpoint detection results should be removed
         $this->image->laserpoints = [
             'area' => 100,
@@ -169,14 +122,5 @@ public function testHandleFatalError()
         ];
 
         $this->assertEquals($expect, $this->image->fresh()->laserpoints);
-        $this->assertFalse($disk->exists($this->gatherFile));
-    }
-
-    public function testFailed()
-    {
-        $disk = Storage::disk('laserpoints');
-        $disk->put($this->gatherFile, 'test');
-        with(new ProcessDelphiJob($this->image, 30, $this->gatherFile))->failed();
-        $this->assertFalse($disk->exists($this->gatherFile));
     }
 }

tests/Jobs/ProcessImageDelphiJobTest.php

@@ -4,16 +4,15 @@
 
 use Biigle\Modules\Laserpoints\Jobs\ProcessDelphiJob;
 use Biigle\Modules\Laserpoints\Jobs\ProcessImageDelphiJob;
-use Biigle\Modules\Laserpoints\Support\DelphiGather;
 use Biigle\Shape;
 use Biigle\Tests\ImageAnnotationLabelTest;
 use Biigle\Tests\ImageAnnotationTest;
 use Biigle\Tests\ImageTest;
 use Biigle\Tests\LabelTest;
-use Mockery;
-use Queue;
 use Storage;
 use TestCase;
+use Illuminate\Bus\PendingBatch;
+use Illuminate\Support\Facades\Bus;
 
 class ProcessImageDelphiJobTest extends TestCase
 {
@@ -39,10 +38,14 @@ public function testHandle()
             'volume_id' => $image->volume_id,
         ]);
 
-        Queue::fake();
+        Bus::fake();
         $job = new ProcessImageDelphiJobStub($image2, 50, $label->id);
         $job->handle();
-        Queue::assertPushed(ProcessDelphiJob::class);
+
+        Bus::assertBatched(function (PendingBatch $batch) {
+            return $batch->jobs->count() === 1 && $batch->jobs[0] instanceof ProcessDelphiJob;
+        });
+
         $points = $job->points->toArray();
         $this->assertArrayHasKey($image->id, $points);
         $points = $points[$image->id];

tests/Jobs/ProcessVolumeDelphiJobTest.php

@@ -11,12 +11,11 @@
 use Biigle\Tests\LabelTest;
 use Biigle\Tests\Modules\Laserpoints\ImageTest as LpImageTest;
 use Biigle\Tests\VolumeTest;
-use Cache;
 use FileCache;
-use Mockery;
-use Queue;
 use Storage;
 use TestCase;
+use Illuminate\Bus\PendingBatch;
+use Illuminate\Support\Facades\Bus;
 
 class ProcessVolumeDelphiJobTest extends TestCase
 {
@@ -37,38 +36,22 @@ public function testHandle()
             'volume_id' => $image->volume_id,
         ]);
 
-        Cache::shouldReceive('rememberForever')->andReturn(Shape::whereName('Point')->first());
-        Cache::shouldReceive('forever')->once()->with(Mockery::any(), 1);
+        Bus::fake();
 
-        Queue::fake();
         $job = new ProcessVolumeDelphiJobStub($image->volume, 50, $label->id);
         $job->handle();
-        Queue::assertPushed(ProcessDelphiJob::class, 1);
-        Queue::assertPushed(ProcessManualJob::class);
+
+
         $expect = [$image->id => '[[0,0],[0,0],[0,0]]'];
         $this->assertEquals($expect, $job->points->toArray());
-    }
-
-    public function testCacheKey()
-    {
-        $label = LabelTest::create();
-        $volume = VolumeTest::create();
-        for ($i = 0; $i < 2; $i++) {
-            $this->createAnnotatedImage($label, $volume->id);
-            ImageTest::create([
-                'volume_id' => $volume->id,
-                'filename' => uniqid(),
-            ]);
-        }
 
-        Cache::shouldReceive('rememberForever')->andReturn(Shape::whereName('Point')->first());
-        Cache::shouldReceive('forever')->once()->with(Mockery::any(), 2);
+        Bus::assertBatched(function (PendingBatch $batch) {
+            return $batch->jobs->count() === 1 && $batch->jobs[0] instanceof ProcessDelphiJob;
+        });
 
-        Queue::fake();
-        $job = new ProcessVolumeDelphiJobStub($volume, 50, $label->id);
-        $job->handle();
-        Queue::assertPushed(ProcessDelphiJob::class, 2);
-        Queue::assertPushed(ProcessManualJob::class);
+        Bus::assertBatched(function (PendingBatch $batch) {
+            return $batch->jobs->count() === 1 && $batch->jobs[0] instanceof ProcessManualJob;
+        });
     }
 
     protected function createAnnotatedImage($label, $volumeId = null)