Merge pull request #967 from biigle/streamed-annotation-response

Streamed annotation response
biigle · Nov 27, 2024 · 47c52ad · 47c52ad
2 parents 2dbacfa + c862628
commit 47c52ad
Show file tree

Hide file tree

Showing 6 changed files with 149 additions and 46 deletions.
diff --git a/app/AnnotationSession.php b/app/AnnotationSession.php
@@ -4,6 +4,7 @@
 
 use Carbon\Carbon;
 use DB;
+use Generator;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 
@@ -64,10 +65,11 @@ public function users()
      *
      * @param VolumeFile $file The file to get the annotations from
      * @param User $user The user to whom the restrictions should apply ('own' user)
+     * @param array $load Models that should also be loaded
      *
-     * @return \Illuminate\Database\Eloquent\Collection
+     * @return Generator
      */
-    public function getVolumeFileAnnotations(VolumeFile $file, User $user)
+    public function getVolumeFileAnnotations(VolumeFile $file, User $user, array $load = [])
     {
         $annotationClass = $file->annotations()->getRelated();
         $query = $annotationClass::allowedBySession($this, $user)
@@ -105,7 +107,14 @@ public function getVolumeFileAnnotations(VolumeFile $file, User $user)
             $query->with('labels');
         }
 
-        return $query->get();
+        // Prevent exceeding memory limit by using generator
+        $yieldAnnotations = function () use ($query, $load): Generator {
+            foreach ($query->with($load)->lazy() as $annotation) {
+                yield $annotation;
+            }
+        };
+
+        return $yieldAnnotations;
     }
 
     /**

diff --git a/app/Http/Controllers/Api/ImageAnnotationController.php b/app/Http/Controllers/Api/ImageAnnotationController.php
@@ -10,8 +10,10 @@
 use Biigle\Shape;
 use DB;
 use Exception;
+use Generator;
 use Illuminate\Http\Request;
 use Illuminate\Validation\ValidationException;
+use Symfony\Component\HttpFoundation\StreamedJsonResponse;
 
 class ImageAnnotationController extends Controller
 {
@@ -60,7 +62,7 @@ class ImageAnnotationController extends Controller
      *
      * @param Request $request
      * @param int $id image id
-     * @return \Illuminate\Database\Eloquent\Collection<int, ImageAnnotation>
+     * @return \Symfony\Component\HttpFoundation\StreamedJsonResponse
      */
     public function index(Request $request, $id)
     {
@@ -78,11 +80,18 @@ public function index(Request $request, $id)
             'labels.user:id,firstname,lastname',
         ];
 
+        // Prevent exceeding memory limit by using generator and stream
         if ($session) {
-            return $session->getVolumeFileAnnotations($image, $user)->load($load);
+            $yieldAnnotations = $session->getVolumeFileAnnotations($image, $user, $load);
+        } else {
+            $yieldAnnotations = function () use ($image, $load): Generator {
+                foreach ($image->annotations()->with($load)->lazy() as $annotation) {
+                    yield $annotation;
+                }
+            };
         }
-
-        return $image->annotations()->with($load)->get();
+    
+        return new StreamedJsonResponse($yieldAnnotations());
     }
 
     /**

diff --git a/app/Http/Controllers/Api/VideoAnnotationController.php b/app/Http/Controllers/Api/VideoAnnotationController.php
@@ -12,9 +12,11 @@
 use Cache;
 use DB;
 use Exception;
+use Generator;
 use Illuminate\Http\Request;
 use Illuminate\Validation\ValidationException;
 use Queue;
+use Symfony\Component\HttpFoundation\StreamedJsonResponse;
 use Symfony\Component\HttpKernel\Exception\TooManyRequestsHttpException;
 
 class VideoAnnotationController extends Controller
@@ -61,7 +63,7 @@ class VideoAnnotationController extends Controller
      *
      * @param Request $request
      * @param int $id Video id
-     * @return mixed
+     * @return \Symfony\Component\HttpFoundation\StreamedJsonResponse
      */
     public function index(Request $request, $id)
     {
@@ -72,11 +74,18 @@ public function index(Request $request, $id)
         $session = $video->volume->getActiveAnnotationSession($user);
         $load = ['labels.label', 'labels.user'];
 
+        // Prevent exceeding memory limit by using generator and stream
         if ($session) {
-            return $session->getVolumeFileAnnotations($video, $user)->load($load);
+            $yieldAnnotations = $session->getVolumeFileAnnotations($video, $user, $load);
+        } else {
+            $yieldAnnotations = function () use ($video, $load): Generator {
+                foreach ($video->annotations()->with($load)->lazy() as $annotation) {
+                    yield $annotation;
+                }
+            };
         }
 
-        return $video->annotations()->with($load)->get();
+        return new StreamedJsonResponse($yieldAnnotations());
     }
 
     /**

diff --git a/tests/php/AnnotationSessionTest.php b/tests/php/AnnotationSessionTest.php
@@ -114,9 +114,9 @@ public function testGetVolumeFileAnnotationsHideOwnImage()
             'hide_other_users_annotations' => false,
         ]);
 
-        $annotations = $session->getVolumeFileAnnotations($image, $ownUser);
+        $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser);
         // expand the models in the collection so we can make assertions
-        $annotations = collect($annotations->toArray());
+        $annotations = collect(collect($yieldAnnotations())->toArray());
 
         $this->assertTrue($annotations->contains('points', [20, 30, 40]));
         $this->assertFalse($annotations->contains('labels', [$al11->toArray()]));
@@ -169,9 +169,9 @@ public function testGetVolumeFileAnnotationsHideOwnVideo()
             'hide_other_users_annotations' => false,
         ]);
 
-        $annotations = $session->getVolumeFileAnnotations($video, $ownUser);
+        $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser);
         // expand the models in the collection so we can make assertions
-        $annotations = collect($annotations->toArray());
+        $annotations = collect(collect($yieldAnnotations())->toArray());
 
         $this->assertTrue($annotations->contains('points', [[20, 30, 40]]));
         $this->assertFalse($annotations->contains('labels', [$al11->toArray()]));
@@ -212,9 +212,9 @@ public function testGetVolumeFileAnnotationsHideOtherImage()
             'hide_other_users_annotations' => true,
         ]);
 
-        $annotations = $session->getVolumeFileAnnotations($image, $ownUser);
+        $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser);
         // expand the models in the collection so we can make assertions
-        $annotations = collect($annotations->toArray());
+        $annotations = collect(collect($yieldAnnotations())->toArray());
 
         $this->assertTrue($annotations->contains('points', [20, 30, 40]));
         $this->assertFalse($annotations->contains('labels', [$al1->toArray()]));
@@ -252,9 +252,9 @@ public function testGetVolumeFileAnnotationsHideOtherVideo()
             'hide_other_users_annotations' => true,
         ]);
 
-        $annotations = $session->getVolumeFileAnnotations($video, $ownUser);
+        $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser);
         // expand the models in the collection so we can make assertions
-        $annotations = collect($annotations->toArray());
+        $annotations = collect(collect($yieldAnnotations())->toArray());
 
         $this->assertTrue($annotations->contains('points', [[20, 30, 40]]));
         $this->assertFalse($annotations->contains('labels', [$al1->toArray()]));
@@ -292,9 +292,9 @@ public function testGetVolumeFileAnnotationsHideBothImage()
             'hide_other_users_annotations' => true,
         ]);
 
-        $annotations = $session->getVolumeFileAnnotations($image, $ownUser);
+        $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser);
         // expand the models in the collection so we can make assertions
-        $annotations = collect($annotations->toArray());
+        $annotations = collect(collect($yieldAnnotations())->toArray());
 
         $this->assertTrue($annotations->contains('points', [40, 50, 60]));
         $this->assertTrue($annotations->contains('labels', [$al1->toArray()]));
@@ -332,9 +332,9 @@ public function testGetVolumeFileAnnotationsHideBothVideo()
             'hide_other_users_annotations' => true,
         ]);
 
-        $annotations = $session->getVolumeFileAnnotations($video, $ownUser);
+        $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser);
         // expand the models in the collection so we can make assertions
-        $annotations = collect($annotations->toArray());
+        $annotations = collect(collect($yieldAnnotations())->toArray());
 
         $this->assertTrue($annotations->contains('points', [[40, 50, 60]]));
         $this->assertTrue($annotations->contains('labels', [$al1->toArray()]));
@@ -371,9 +371,9 @@ public function testGetVolumeFileAnnotationsHideNothingImage()
             'hide_other_users_annotations' => false,
         ]);
 
-        $annotations = $session->getVolumeFileAnnotations($image, $ownUser);
+        $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser);
         // expand the models in the collection so we can make assertions
-        $annotations = collect($annotations->toArray());
+        $annotations = collect(collect($yieldAnnotations())->toArray());
 
         $this->assertTrue($annotations->contains('points', [40, 50, 60]));
         $this->assertTrue($annotations->contains('labels', [
@@ -412,9 +412,9 @@ public function testGetVolumeFileAnnotationsHideNothingVideo()
             'hide_other_users_annotations' => false,
         ]);
 
-        $annotations = $session->getVolumeFileAnnotations($video, $ownUser);
+        $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser);
         // expand the models in the collection so we can make assertions
-        $annotations = collect($annotations->toArray());
+        $annotations = collect(collect($yieldAnnotations())->toArray());
 
         $this->assertTrue($annotations->contains('points', [[40, 50, 60]]));
         $this->assertTrue($annotations->contains('labels', [

diff --git a/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php b/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php
@@ -11,6 +11,8 @@
 use Biigle\Tests\LabelTest;
 use Cache;
 use Carbon\Carbon;
+use Illuminate\Testing\TestResponse;
+use Symfony\Component\HttpFoundation\Response;
 
 class ImageAnnotationControllerTest extends ApiTestCase
 {
@@ -49,11 +51,23 @@ public function testIndex()
         $response->assertStatus(403);
 
         $this->beGuest();
-        $response = $this->get("/api/v1/images/{$this->image->id}/annotations")
-            ->assertJsonFragment(['points' => [10, 20, 30, 40]])
+        $response = $this->getJson("/api/v1/images/{$this->image->id}/annotations")->assertStatus(200);
+
+        ob_start();
+        $response->sendContent();
+        $content = ob_get_clean();
+        $response = new TestResponse(
+            new Response(
+                $content,
+                $response->baseResponse->getStatusCode(),
+                $response->baseResponse->headers->all()
+            )
+        );
+
+        $response->assertJsonFragment(['points' => [10, 20, 30, 40]])
             ->assertJsonFragment(['color' => 'bada55'])
             ->assertJsonFragment(['name' => 'My label']);
-        $response->assertStatus(200);
+
     }
 
     public function testIndexAnnotationSessionHideOwn()
@@ -89,18 +103,42 @@ public function testIndexAnnotationSessionHideOwn()
         ]);
 
         $this->beEditor();
-        $response = $this->get("/api/v1/images/{$this->image->id}/annotations")
-            ->assertJsonFragment(['points' => [10, 20]])
+        $response = $this->getJson("/api/v1/images/{$this->image->id}/annotations")->assertStatus(200);
+
+        ob_start();
+        $response->sendContent();
+        $content = ob_get_clean();
+        $response = new TestResponse(
+            new Response(
+                $content,
+                $response->baseResponse->getStatusCode(),
+                $response->baseResponse->headers->all()
+            )
+        );
+
+        $response->assertJsonFragment(['points' => [10, 20]])
             ->assertJsonFragment(['points' => [20, 30]]);
-        $response->assertStatus(200);
+
 
         $session->users()->attach($this->editor());
         Cache::flush();
 
-        $response = $this->get("/api/v1/images/{$this->image->id}/annotations")
-            ->assertJsonMissing(['points' => [10, 20]])
+        $response = $this->getJson("/api/v1/images/{$this->image->id}/annotations")->assertStatus(200);
+
+        ob_start();
+        $response->sendContent();
+        $content = ob_get_clean();
+        $response = new TestResponse(
+            new Response(
+                $content,
+                $response->baseResponse->getStatusCode(),
+                $response->baseResponse->headers->all()
+            )
+        );
+
+        $response->assertJsonMissing(['points' => [10, 20]])
             ->assertJsonFragment(['points' => [20, 30]]);
-        $response->assertStatus(200);
+
     }
 
     public function testShow()

diff --git a/tests/php/Http/Controllers/Api/VideoAnnotationControllerTest.php b/tests/php/Http/Controllers/Api/VideoAnnotationControllerTest.php
@@ -13,7 +13,9 @@
 use Biigle\Tests\VideoTest;
 use Cache;
 use Carbon\Carbon;
+use Illuminate\Testing\TestResponse;
 use Queue;
+use Symfony\Component\HttpFoundation\Response;
 
 class VideoAnnotationControllerTest extends ApiTestCase
 {
@@ -54,10 +56,22 @@ public function testIndex()
             ->assertStatus(403);
 
         $this->beGuest();
-        $this
+        $response = $this
             ->getJson("/api/v1/videos/{$this->video->id}/annotations")
-            ->assertStatus(200)
-            ->assertJsonFragment(['frames' => [1.0]])
+            ->assertStatus(200);
+
+        ob_start();
+        $response->sendContent();
+        $content = ob_get_clean();
+        $response = new TestResponse(
+            new Response(
+                $content,
+                $response->baseResponse->getStatusCode(),
+                $response->baseResponse->headers->all()
+            )
+        );
+
+        $response->assertJsonFragment(['frames' => [1.0]])
             ->assertJsonFragment(['points' => [[10, 20]]])
             ->assertJsonFragment(['color' => 'bada55'])
             ->assertJsonFragment(['name' => 'My label']);
@@ -96,19 +110,43 @@ public function testIndexAnnotationSessionHideOwn()
         ]);
 
         $this->beEditor();
-        $this
-            ->get("/api/v1/videos/{$this->video->id}/annotations")
-            ->assertStatus(200)
-            ->assertJsonFragment(['points' => [[10, 20]]])
+        $response = $this
+            ->getJson("/api/v1/videos/{$this->video->id}/annotations")
+            ->assertStatus(200);
+
+        ob_start();
+        $response->sendContent();
+        $content = ob_get_clean();
+        $response = new TestResponse(
+            new Response(
+                $content,
+                $response->baseResponse->getStatusCode(),
+                $response->baseResponse->headers->all()
+            )
+        );
+
+        $response->assertJsonFragment(['points' => [[10, 20]]])
             ->assertJsonFragment(['points' => [[20, 30]]]);
 
         $session->users()->attach($this->editor());
         Cache::flush();
 
-        $this
-            ->get("/api/v1/videos/{$this->video->id}/annotations")
-            ->assertStatus(200)
-            ->assertJsonMissing(['points' => [[10, 20]]])
+        $response = $this
+            ->getJson("/api/v1/videos/{$this->video->id}/annotations")
+            ->assertStatus(200);
+
+        ob_start();
+        $response->sendContent();
+        $content = ob_get_clean();
+        $response = new TestResponse(
+            new Response(
+                $content,
+                $response->baseResponse->getStatusCode(),
+                $response->baseResponse->headers->all()
+            )
+        );
+
+        $response->assertJsonMissing(['points' => [[10, 20]]])
             ->assertJsonFragment(['points' => [[20, 30]]]);
     }