From 5662d6cb281561d8ac19ba8afab3d86655884a7e Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Wed, 6 Nov 2024 07:59:31 +0100 Subject: [PATCH 1/7] Stream annotations from generator Prevent exceeding memory limit by using stream and generator. --- .../Api/ImageAnnotationController.php | 23 ++++-- .../Api/VideoAnnotationController.php | 27 ++++--- .../Api/ImageAnnotationControllerTest.php | 63 ++++++++++++---- .../Api/VideoAnnotationControllerTest.php | 73 ++++++++++++++----- 4 files changed, 135 insertions(+), 51 deletions(-) diff --git a/app/Http/Controllers/Api/ImageAnnotationController.php b/app/Http/Controllers/Api/ImageAnnotationController.php index 64461ec36..d26f3d43b 100644 --- a/app/Http/Controllers/Api/ImageAnnotationController.php +++ b/app/Http/Controllers/Api/ImageAnnotationController.php @@ -2,15 +2,16 @@ namespace Biigle\Http\Controllers\Api; -use Biigle\Http\Requests\StoreImageAnnotation; +use DB; +use Exception; +use Generator; use Biigle\Image; -use Biigle\ImageAnnotation; -use Biigle\ImageAnnotationLabel; use Biigle\Label; use Biigle\Shape; -use DB; -use Exception; +use Biigle\ImageAnnotation; use Illuminate\Http\Request; +use Biigle\ImageAnnotationLabel; +use Biigle\Http\Requests\StoreImageAnnotation; use Illuminate\Validation\ValidationException; class ImageAnnotationController extends Controller @@ -79,10 +80,16 @@ public function index(Request $request, $id) ]; 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 response()->streamJson($yieldAnnotations()); } /** diff --git a/app/Http/Controllers/Api/VideoAnnotationController.php b/app/Http/Controllers/Api/VideoAnnotationController.php index c9d9a7334..a75684e7b 100644 --- a/app/Http/Controllers/Api/VideoAnnotationController.php +++ b/app/Http/Controllers/Api/VideoAnnotationController.php @@ -2,19 +2,20 @@ namespace Biigle\Http\Controllers\Api; -use Biigle\Http\Requests\StoreVideoAnnotation; -use Biigle\Http\Requests\UpdateVideoAnnotation; -use Biigle\Jobs\TrackObject; +use DB; +use Cache; +use Queue; +use Exception; +use Generator; use Biigle\Label; use Biigle\Video; use Biigle\VideoAnnotation; -use Biigle\VideoAnnotationLabel; -use Cache; -use DB; -use Exception; +use Biigle\Jobs\TrackObject; use Illuminate\Http\Request; +use Biigle\VideoAnnotationLabel; +use Biigle\Http\Requests\StoreVideoAnnotation; use Illuminate\Validation\ValidationException; -use Queue; +use Biigle\Http\Requests\UpdateVideoAnnotation; use Symfony\Component\HttpKernel\Exception\TooManyRequestsHttpException; class VideoAnnotationController extends Controller @@ -73,10 +74,16 @@ public function index(Request $request, $id) $load = ['labels.label', 'labels.user']; 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 response()->streamJson($yieldAnnotations()); } /** diff --git a/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php b/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php index 1ede2435e..1b6411b67 100644 --- a/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php +++ b/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php @@ -2,15 +2,17 @@ namespace Biigle\Tests\Http\Controllers\Api; +use Cache; use ApiTestCase; use Biigle\Shape; -use Biigle\Tests\AnnotationSessionTest; -use Biigle\Tests\ImageAnnotationLabelTest; -use Biigle\Tests\ImageAnnotationTest; +use Carbon\Carbon; use Biigle\Tests\ImageTest; use Biigle\Tests\LabelTest; -use Cache; -use Carbon\Carbon; +use Illuminate\Testing\TestResponse; +use Biigle\Tests\ImageAnnotationTest; +use Biigle\Tests\AnnotationSessionTest; +use Biigle\Tests\ImageAnnotationLabelTest; +use Symfony\Component\HttpFoundation\Response; class ImageAnnotationControllerTest extends ApiTestCase { @@ -49,11 +51,22 @@ 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 +102,40 @@ 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 index 69cba4f5f..128fb771f 100644 --- a/tests/php/Http/Controllers/Api/VideoAnnotationControllerTest.php +++ b/tests/php/Http/Controllers/Api/VideoAnnotationControllerTest.php @@ -2,18 +2,20 @@ namespace Biigle\Tests\Http\Controllers\Api; +use Cache; +use Queue; use ApiTestCase; -use Biigle\Jobs\TrackObject; -use Biigle\MediaType; use Biigle\Shape; -use Biigle\Tests\AnnotationSessionTest; +use Carbon\Carbon; +use Biigle\MediaType; use Biigle\Tests\LabelTest; -use Biigle\Tests\VideoAnnotationLabelTest; -use Biigle\Tests\VideoAnnotationTest; use Biigle\Tests\VideoTest; -use Cache; -use Carbon\Carbon; -use Queue; +use Biigle\Jobs\TrackObject; +use Illuminate\Testing\TestResponse; +use Biigle\Tests\VideoAnnotationTest; +use Biigle\Tests\AnnotationSessionTest; +use Biigle\Tests\VideoAnnotationLabelTest; +use Symfony\Component\HttpFoundation\Response; class VideoAnnotationControllerTest extends ApiTestCase { @@ -54,10 +56,21 @@ 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 +109,41 @@ 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]]]); } From 64424494010926b3431e46ccd696a573b2fdc911 Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Thu, 7 Nov 2024 07:59:05 +0100 Subject: [PATCH 2/7] Return an annotation generator for annotation sessions Prevent exceeding memory limit by using generator. --- app/AnnotationSession.php | 15 +++-- tests/php/AnnotationSessionTest.php | 99 ++++++++++++++++------------- 2 files changed, 65 insertions(+), 49 deletions(-) diff --git a/app/AnnotationSession.php b/app/AnnotationSession.php index 26bf601fc..abf9cca04 100644 --- a/app/AnnotationSession.php +++ b/app/AnnotationSession.php @@ -2,10 +2,11 @@ namespace Biigle; -use Carbon\Carbon; use DB; -use Illuminate\Database\Eloquent\Factories\HasFactory; +use Generator; +use Carbon\Carbon; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Factories\HasFactory; /** * An annotation session groups multiple annotations of a volume based on their @@ -67,7 +68,7 @@ public function users() * * @return \Illuminate\Database\Eloquent\Collection */ - 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 +106,13 @@ public function getVolumeFileAnnotations(VolumeFile $file, User $user) $query->with('labels'); } - return $query->get(); + $yieldAnnotations = function () use ($query, $load): Generator { + foreach ($query->with($load)->lazy() as $annotation) { + yield $annotation; + } + }; + + return $yieldAnnotations; } /** diff --git a/tests/php/AnnotationSessionTest.php b/tests/php/AnnotationSessionTest.php index d1b3869b9..490f80b0a 100644 --- a/tests/php/AnnotationSessionTest.php +++ b/tests/php/AnnotationSessionTest.php @@ -2,11 +2,12 @@ namespace Biigle\Tests; -use Biigle\AnnotationSession; -use Biigle\MediaType; use Carbon\Carbon; -use Illuminate\Database\QueryException; use ModelTestCase; +use Biigle\MediaType; +use Biigle\AnnotationSession; +use Illuminate\Support\Facades\Log; +use Illuminate\Database\QueryException; class AnnotationSessionTest extends ModelTestCase { @@ -91,7 +92,7 @@ public function testGetVolumeFileAnnotationsHideOwnImage() $al12 = ImageAnnotationLabelTest::create([ 'annotation_id' => $a1->id, 'user_id' => $otherUser->id, - 'created_at' => '2016-09-05', + 'created_at' => '2022-09-05', ]); // this should be shown completely @@ -114,16 +115,16 @@ 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($yieldAnnotations()); + $labels = $annotations->pluck('labels'); - $this->assertTrue($annotations->contains('points', [20, 30, 40])); - $this->assertFalse($annotations->contains('labels', [$al11->toArray()])); - $this->assertTrue($annotations->contains('labels', [$al12->toArray()])); + $this->assertFalse($this->isPresent($al11, $labels)); + $this->assertTrue($this->isPresent($al12, $labels)); $this->assertTrue($annotations->contains('points', [30, 40, 50])); - $this->assertTrue($annotations->contains('labels', [$al2->toArray()])); + $this->assertTrue($this->isPresent($al2, $labels)); } public function testGetVolumeFileAnnotationsHideOwnVideo() @@ -169,16 +170,16 @@ 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()); - - $this->assertTrue($annotations->contains('points', [[20, 30, 40]])); - $this->assertFalse($annotations->contains('labels', [$al11->toArray()])); - $this->assertTrue($annotations->contains('labels', [$al12->toArray()])); + $annotations = collect($yieldAnnotations()); + $labels = $annotations->pluck('labels'); + + $this->assertFalse($this->isPresent($al11, $labels)); + $this->assertTrue($this->isPresent($al12, $labels)); $this->assertTrue($annotations->contains('points', [[30, 40, 50]])); - $this->assertTrue($annotations->contains('labels', [$al2->toArray()])); + $this->assertTrue($this->isPresent($al2, $labels)); } public function testGetVolumeFileAnnotationsHideOtherImage() @@ -212,13 +213,14 @@ 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($yieldAnnotations()); + $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [20, 30, 40])); - $this->assertFalse($annotations->contains('labels', [$al1->toArray()])); - $this->assertTrue($annotations->contains('labels', [$al2->toArray()])); + $this->assertFalse($this->isPresent($al1, $labels)); + $this->assertTrue($this->isPresent($al2, $labels)); } public function testGetVolumeFileAnnotationsHideOtherVideo() @@ -252,13 +254,14 @@ 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($yieldAnnotations()); + $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [[20, 30, 40]])); - $this->assertFalse($annotations->contains('labels', [$al1->toArray()])); - $this->assertTrue($annotations->contains('labels', [$al2->toArray()])); + $this->assertFalse($this->isPresent($al1, $labels)); + $this->assertTrue($this->isPresent($al2, $labels)); } public function testGetVolumeFileAnnotationsHideBothImage() @@ -292,13 +295,14 @@ 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($yieldAnnotations()); + $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [40, 50, 60])); - $this->assertTrue($annotations->contains('labels', [$al1->toArray()])); - $this->assertFalse($annotations->contains('labels', [$al2->toArray()])); + $this->assertTrue($this->isPresent($al1, $labels)); + $this->assertFalse($this->isPresent($al2, $labels)); } public function testGetVolumeFileAnnotationsHideBothVideo() @@ -332,13 +336,14 @@ 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($yieldAnnotations()); + $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [[40, 50, 60]])); - $this->assertTrue($annotations->contains('labels', [$al1->toArray()])); - $this->assertFalse($annotations->contains('labels', [$al2->toArray()])); + $this->assertTrue($this->isPresent($al1, $labels)); + $this->assertFalse($this->isPresent($al2, $labels)); } public function testGetVolumeFileAnnotationsHideNothingImage() @@ -371,15 +376,14 @@ 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($yieldAnnotations()); + $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [40, 50, 60])); - $this->assertTrue($annotations->contains('labels', [ - $al1->toArray(), - $al2->toArray(), - ])); + $this->assertTrue($this->isPresent($al1, $labels)); + $this->assertTrue($this->isPresent($al2, $labels)); } public function testGetVolumeFileAnnotationsHideNothingVideo() @@ -412,15 +416,14 @@ 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($yieldAnnotations()); + $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [[40, 50, 60]])); - $this->assertTrue($annotations->contains('labels', [ - $al1->toArray(), - $al2->toArray(), - ])); + $this->assertTrue($this->isPresent($al1, $labels)); + $this->assertTrue($this->isPresent($al2, $labels)); } public function testAllowsAccessImageAnnotation() @@ -732,4 +735,10 @@ public function testAnnotationsVideo() $this->assertFalse($session->annotations()->where('id', $a3->id)->exists()); $this->assertFalse($session->annotations()->where('id', $a4->id)->exists()); } + + private function isPresent($needle, $haystack){ + $needle = collect($needle)->sortKeys(); + $haystack = $haystack->flatten()->map(fn($l) => collect($l)->sortKeys()); + return $haystack->contains($needle); + } } From abb8eda1d814f5e970fffcdd47c6e216060d35c4 Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Thu, 7 Nov 2024 08:17:33 +0100 Subject: [PATCH 3/7] Apply composer fix changes --- app/AnnotationSession.php | 4 +-- .../Api/ImageAnnotationController.php | 12 +++---- .../Api/VideoAnnotationController.php | 18 +++++----- tests/php/AnnotationSessionTest.php | 36 +++++++++---------- .../Api/ImageAnnotationControllerTest.php | 19 +++++----- .../Api/VideoAnnotationControllerTest.php | 25 +++++++------ 6 files changed, 60 insertions(+), 54 deletions(-) diff --git a/app/AnnotationSession.php b/app/AnnotationSession.php index abf9cca04..19760b916 100644 --- a/app/AnnotationSession.php +++ b/app/AnnotationSession.php @@ -2,11 +2,11 @@ namespace Biigle; +use Carbon\Carbon; use DB; use Generator; -use Carbon\Carbon; -use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Factories\HasFactory; +use Illuminate\Database\Eloquent\Model; /** * An annotation session groups multiple annotations of a volume based on their diff --git a/app/Http/Controllers/Api/ImageAnnotationController.php b/app/Http/Controllers/Api/ImageAnnotationController.php index d26f3d43b..af932bd2c 100644 --- a/app/Http/Controllers/Api/ImageAnnotationController.php +++ b/app/Http/Controllers/Api/ImageAnnotationController.php @@ -2,16 +2,16 @@ namespace Biigle\Http\Controllers\Api; -use DB; -use Exception; -use Generator; +use Biigle\Http\Requests\StoreImageAnnotation; use Biigle\Image; +use Biigle\ImageAnnotation; +use Biigle\ImageAnnotationLabel; use Biigle\Label; use Biigle\Shape; -use Biigle\ImageAnnotation; +use DB; +use Exception; +use Generator; use Illuminate\Http\Request; -use Biigle\ImageAnnotationLabel; -use Biigle\Http\Requests\StoreImageAnnotation; use Illuminate\Validation\ValidationException; class ImageAnnotationController extends Controller diff --git a/app/Http/Controllers/Api/VideoAnnotationController.php b/app/Http/Controllers/Api/VideoAnnotationController.php index a75684e7b..e93c209ac 100644 --- a/app/Http/Controllers/Api/VideoAnnotationController.php +++ b/app/Http/Controllers/Api/VideoAnnotationController.php @@ -2,20 +2,20 @@ namespace Biigle\Http\Controllers\Api; -use DB; -use Cache; -use Queue; -use Exception; -use Generator; +use Biigle\Http\Requests\StoreVideoAnnotation; +use Biigle\Http\Requests\UpdateVideoAnnotation; +use Biigle\Jobs\TrackObject; use Biigle\Label; use Biigle\Video; use Biigle\VideoAnnotation; -use Biigle\Jobs\TrackObject; -use Illuminate\Http\Request; use Biigle\VideoAnnotationLabel; -use Biigle\Http\Requests\StoreVideoAnnotation; +use Cache; +use DB; +use Exception; +use Generator; +use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; -use Biigle\Http\Requests\UpdateVideoAnnotation; +use Queue; use Symfony\Component\HttpKernel\Exception\TooManyRequestsHttpException; class VideoAnnotationController extends Controller diff --git a/tests/php/AnnotationSessionTest.php b/tests/php/AnnotationSessionTest.php index 490f80b0a..fb37bfb3a 100644 --- a/tests/php/AnnotationSessionTest.php +++ b/tests/php/AnnotationSessionTest.php @@ -2,12 +2,11 @@ namespace Biigle\Tests; -use Carbon\Carbon; -use ModelTestCase; -use Biigle\MediaType; use Biigle\AnnotationSession; -use Illuminate\Support\Facades\Log; +use Biigle\MediaType; +use Carbon\Carbon; use Illuminate\Database\QueryException; +use ModelTestCase; class AnnotationSessionTest extends ModelTestCase { @@ -172,7 +171,7 @@ public function testGetVolumeFileAnnotationsHideOwnVideo() $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); + $annotations = collect($yieldAnnotations()); $labels = $annotations->pluck('labels'); $this->assertFalse($this->isPresent($al11, $labels)); @@ -213,9 +212,9 @@ public function testGetVolumeFileAnnotationsHideOtherImage() 'hide_other_users_annotations' => true, ]); - $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser);; + $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); + $annotations = collect($yieldAnnotations()); $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [20, 30, 40])); @@ -254,9 +253,9 @@ public function testGetVolumeFileAnnotationsHideOtherVideo() 'hide_other_users_annotations' => true, ]); - $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser);; + $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); + $annotations = collect($yieldAnnotations()); $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [[20, 30, 40]])); @@ -295,9 +294,9 @@ public function testGetVolumeFileAnnotationsHideBothImage() 'hide_other_users_annotations' => true, ]); - $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser);; + $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); + $annotations = collect($yieldAnnotations()); $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [40, 50, 60])); @@ -336,9 +335,9 @@ public function testGetVolumeFileAnnotationsHideBothVideo() 'hide_other_users_annotations' => true, ]); - $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser);; + $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); + $annotations = collect($yieldAnnotations()); $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [[40, 50, 60]])); @@ -378,7 +377,7 @@ public function testGetVolumeFileAnnotationsHideNothingImage() $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); + $annotations = collect($yieldAnnotations()); $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [40, 50, 60])); @@ -416,9 +415,9 @@ public function testGetVolumeFileAnnotationsHideNothingVideo() 'hide_other_users_annotations' => false, ]); - $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser);; + $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); + $annotations = collect($yieldAnnotations()); $labels = $annotations->pluck('labels'); $this->assertTrue($annotations->contains('points', [[40, 50, 60]])); @@ -736,9 +735,10 @@ public function testAnnotationsVideo() $this->assertFalse($session->annotations()->where('id', $a4->id)->exists()); } - private function isPresent($needle, $haystack){ + private function isPresent($needle, $haystack) + { $needle = collect($needle)->sortKeys(); - $haystack = $haystack->flatten()->map(fn($l) => collect($l)->sortKeys()); + $haystack = $haystack->flatten()->map(fn ($l) => collect($l)->sortKeys()); return $haystack->contains($needle); } } diff --git a/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php b/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php index 1b6411b67..3864ccf6a 100644 --- a/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php +++ b/tests/php/Http/Controllers/Api/ImageAnnotationControllerTest.php @@ -2,16 +2,16 @@ namespace Biigle\Tests\Http\Controllers\Api; -use Cache; use ApiTestCase; use Biigle\Shape; -use Carbon\Carbon; +use Biigle\Tests\AnnotationSessionTest; +use Biigle\Tests\ImageAnnotationLabelTest; +use Biigle\Tests\ImageAnnotationTest; use Biigle\Tests\ImageTest; use Biigle\Tests\LabelTest; +use Cache; +use Carbon\Carbon; use Illuminate\Testing\TestResponse; -use Biigle\Tests\ImageAnnotationTest; -use Biigle\Tests\AnnotationSessionTest; -use Biigle\Tests\ImageAnnotationLabelTest; use Symfony\Component\HttpFoundation\Response; class ImageAnnotationControllerTest extends ApiTestCase @@ -57,7 +57,8 @@ public function testIndex() $response->sendContent(); $content = ob_get_clean(); $response = new TestResponse( - new Response($content, + new Response( + $content, $response->baseResponse->getStatusCode(), $response->baseResponse->headers->all() ) @@ -108,7 +109,8 @@ public function testIndexAnnotationSessionHideOwn() $response->sendContent(); $content = ob_get_clean(); $response = new TestResponse( - new Response($content, + new Response( + $content, $response->baseResponse->getStatusCode(), $response->baseResponse->headers->all() ) @@ -127,7 +129,8 @@ public function testIndexAnnotationSessionHideOwn() $response->sendContent(); $content = ob_get_clean(); $response = new TestResponse( - new Response($content, + new Response( + $content, $response->baseResponse->getStatusCode(), $response->baseResponse->headers->all() ) diff --git a/tests/php/Http/Controllers/Api/VideoAnnotationControllerTest.php b/tests/php/Http/Controllers/Api/VideoAnnotationControllerTest.php index 128fb771f..c23d1e1be 100644 --- a/tests/php/Http/Controllers/Api/VideoAnnotationControllerTest.php +++ b/tests/php/Http/Controllers/Api/VideoAnnotationControllerTest.php @@ -2,19 +2,19 @@ namespace Biigle\Tests\Http\Controllers\Api; -use Cache; -use Queue; use ApiTestCase; -use Biigle\Shape; -use Carbon\Carbon; +use Biigle\Jobs\TrackObject; use Biigle\MediaType; +use Biigle\Shape; +use Biigle\Tests\AnnotationSessionTest; use Biigle\Tests\LabelTest; +use Biigle\Tests\VideoAnnotationLabelTest; +use Biigle\Tests\VideoAnnotationTest; use Biigle\Tests\VideoTest; -use Biigle\Jobs\TrackObject; +use Cache; +use Carbon\Carbon; use Illuminate\Testing\TestResponse; -use Biigle\Tests\VideoAnnotationTest; -use Biigle\Tests\AnnotationSessionTest; -use Biigle\Tests\VideoAnnotationLabelTest; +use Queue; use Symfony\Component\HttpFoundation\Response; class VideoAnnotationControllerTest extends ApiTestCase @@ -64,7 +64,8 @@ public function testIndex() $response->sendContent(); $content = ob_get_clean(); $response = new TestResponse( - new Response($content, + new Response( + $content, $response->baseResponse->getStatusCode(), $response->baseResponse->headers->all() ) @@ -117,7 +118,8 @@ public function testIndexAnnotationSessionHideOwn() $response->sendContent(); $content = ob_get_clean(); $response = new TestResponse( - new Response($content, + new Response( + $content, $response->baseResponse->getStatusCode(), $response->baseResponse->headers->all() ) @@ -137,7 +139,8 @@ public function testIndexAnnotationSessionHideOwn() $response->sendContent(); $content = ob_get_clean(); $response = new TestResponse( - new Response($content, + new Response( + $content, $response->baseResponse->getStatusCode(), $response->baseResponse->headers->all() ) From 82d2f5bc6d2a9596ef8bfed765788594d5cbcfd5 Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Thu, 7 Nov 2024 08:22:27 +0100 Subject: [PATCH 4/7] Add comment and update php doc --- app/AnnotationSession.php | 4 +++- app/Http/Controllers/Api/ImageAnnotationController.php | 3 ++- app/Http/Controllers/Api/VideoAnnotationController.php | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/AnnotationSession.php b/app/AnnotationSession.php index 19760b916..f66e12813 100644 --- a/app/AnnotationSession.php +++ b/app/AnnotationSession.php @@ -65,8 +65,9 @@ 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, array $load = []) { @@ -106,6 +107,7 @@ public function getVolumeFileAnnotations(VolumeFile $file, User $user, array $lo $query->with('labels'); } + // Prevent exceeding memory limit by using generator $yieldAnnotations = function () use ($query, $load): Generator { foreach ($query->with($load)->lazy() as $annotation) { yield $annotation; diff --git a/app/Http/Controllers/Api/ImageAnnotationController.php b/app/Http/Controllers/Api/ImageAnnotationController.php index af932bd2c..be57856d7 100644 --- a/app/Http/Controllers/Api/ImageAnnotationController.php +++ b/app/Http/Controllers/Api/ImageAnnotationController.php @@ -61,7 +61,7 @@ class ImageAnnotationController extends Controller * * @param Request $request * @param int $id image id - * @return \Illuminate\Database\Eloquent\Collection + * @return \Symfony\Component\HttpFoundation\StreamedJsonResponse */ public function index(Request $request, $id) { @@ -79,6 +79,7 @@ public function index(Request $request, $id) 'labels.user:id,firstname,lastname', ]; + // Prevent exceeding memory limit by using generator and stream if ($session) { $yieldAnnotations = $session->getVolumeFileAnnotations($image, $user, $load); } else { diff --git a/app/Http/Controllers/Api/VideoAnnotationController.php b/app/Http/Controllers/Api/VideoAnnotationController.php index e93c209ac..68511846c 100644 --- a/app/Http/Controllers/Api/VideoAnnotationController.php +++ b/app/Http/Controllers/Api/VideoAnnotationController.php @@ -62,7 +62,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) { @@ -73,6 +73,7 @@ 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) { $yieldAnnotations = $session->getVolumeFileAnnotations($video, $user, $load); } else { From f040bdf235140e33c78026c8b7c0af25bbd74877 Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Thu, 7 Nov 2024 08:36:03 +0100 Subject: [PATCH 5/7] Apply composer lint changes --- app/Http/Controllers/Api/ImageAnnotationController.php | 2 +- app/Http/Controllers/Api/VideoAnnotationController.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Api/ImageAnnotationController.php b/app/Http/Controllers/Api/ImageAnnotationController.php index be57856d7..b3da9a535 100644 --- a/app/Http/Controllers/Api/ImageAnnotationController.php +++ b/app/Http/Controllers/Api/ImageAnnotationController.php @@ -90,7 +90,7 @@ public function index(Request $request, $id) }; } - return response()->streamJson($yieldAnnotations()); + return response()->streamJson(iterator_to_array($yieldAnnotations())); } /** diff --git a/app/Http/Controllers/Api/VideoAnnotationController.php b/app/Http/Controllers/Api/VideoAnnotationController.php index 68511846c..6f876396d 100644 --- a/app/Http/Controllers/Api/VideoAnnotationController.php +++ b/app/Http/Controllers/Api/VideoAnnotationController.php @@ -84,7 +84,7 @@ public function index(Request $request, $id) }; } - return response()->streamJson($yieldAnnotations()); + return response()->streamJson(iterator_to_array($yieldAnnotations())); } /** From 6199857817bebfdd5b36f068fe4d55f844b16642 Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Tue, 12 Nov 2024 11:31:56 +0100 Subject: [PATCH 6/7] Use StreamedJsonRespone to avoid linting error --- app/Http/Controllers/Api/ImageAnnotationController.php | 3 ++- app/Http/Controllers/Api/VideoAnnotationController.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Api/ImageAnnotationController.php b/app/Http/Controllers/Api/ImageAnnotationController.php index b3da9a535..6a0413183 100644 --- a/app/Http/Controllers/Api/ImageAnnotationController.php +++ b/app/Http/Controllers/Api/ImageAnnotationController.php @@ -13,6 +13,7 @@ use Generator; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; +use Symfony\Component\HttpFoundation\StreamedJsonResponse; class ImageAnnotationController extends Controller { @@ -90,7 +91,7 @@ public function index(Request $request, $id) }; } - return response()->streamJson(iterator_to_array($yieldAnnotations())); + return new StreamedJsonResponse($yieldAnnotations()); } /** diff --git a/app/Http/Controllers/Api/VideoAnnotationController.php b/app/Http/Controllers/Api/VideoAnnotationController.php index 6f876396d..619bd8aaa 100644 --- a/app/Http/Controllers/Api/VideoAnnotationController.php +++ b/app/Http/Controllers/Api/VideoAnnotationController.php @@ -16,6 +16,7 @@ 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 @@ -84,7 +85,7 @@ public function index(Request $request, $id) }; } - return response()->streamJson(iterator_to_array($yieldAnnotations())); + return new StreamedJsonResponse($yieldAnnotations()); } /** From c862628fcba0ffeb2adc6f0df57bd9f51db71886 Mon Sep 17 00:00:00 2001 From: Leane Schlundt Date: Wed, 13 Nov 2024 07:37:59 +0100 Subject: [PATCH 7/7] Revert assertion changes in AnnotationSessionTest --- tests/php/AnnotationSessionTest.php | 77 +++++++++++++---------------- 1 file changed, 34 insertions(+), 43 deletions(-) diff --git a/tests/php/AnnotationSessionTest.php b/tests/php/AnnotationSessionTest.php index fb37bfb3a..74901c26a 100644 --- a/tests/php/AnnotationSessionTest.php +++ b/tests/php/AnnotationSessionTest.php @@ -91,7 +91,7 @@ public function testGetVolumeFileAnnotationsHideOwnImage() $al12 = ImageAnnotationLabelTest::create([ 'annotation_id' => $a1->id, 'user_id' => $otherUser->id, - 'created_at' => '2022-09-05', + 'created_at' => '2016-09-05', ]); // this should be shown completely @@ -116,14 +116,14 @@ public function testGetVolumeFileAnnotationsHideOwnImage() $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); - $labels = $annotations->pluck('labels'); + $annotations = collect(collect($yieldAnnotations())->toArray()); - $this->assertFalse($this->isPresent($al11, $labels)); - $this->assertTrue($this->isPresent($al12, $labels)); + $this->assertTrue($annotations->contains('points', [20, 30, 40])); + $this->assertFalse($annotations->contains('labels', [$al11->toArray()])); + $this->assertTrue($annotations->contains('labels', [$al12->toArray()])); $this->assertTrue($annotations->contains('points', [30, 40, 50])); - $this->assertTrue($this->isPresent($al2, $labels)); + $this->assertTrue($annotations->contains('labels', [$al2->toArray()])); } public function testGetVolumeFileAnnotationsHideOwnVideo() @@ -171,14 +171,14 @@ public function testGetVolumeFileAnnotationsHideOwnVideo() $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); - $labels = $annotations->pluck('labels'); - - $this->assertFalse($this->isPresent($al11, $labels)); - $this->assertTrue($this->isPresent($al12, $labels)); + $annotations = collect(collect($yieldAnnotations())->toArray()); + + $this->assertTrue($annotations->contains('points', [[20, 30, 40]])); + $this->assertFalse($annotations->contains('labels', [$al11->toArray()])); + $this->assertTrue($annotations->contains('labels', [$al12->toArray()])); $this->assertTrue($annotations->contains('points', [[30, 40, 50]])); - $this->assertTrue($this->isPresent($al2, $labels)); + $this->assertTrue($annotations->contains('labels', [$al2->toArray()])); } public function testGetVolumeFileAnnotationsHideOtherImage() @@ -214,12 +214,11 @@ public function testGetVolumeFileAnnotationsHideOtherImage() $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); - $labels = $annotations->pluck('labels'); + $annotations = collect(collect($yieldAnnotations())->toArray()); $this->assertTrue($annotations->contains('points', [20, 30, 40])); - $this->assertFalse($this->isPresent($al1, $labels)); - $this->assertTrue($this->isPresent($al2, $labels)); + $this->assertFalse($annotations->contains('labels', [$al1->toArray()])); + $this->assertTrue($annotations->contains('labels', [$al2->toArray()])); } public function testGetVolumeFileAnnotationsHideOtherVideo() @@ -255,12 +254,11 @@ public function testGetVolumeFileAnnotationsHideOtherVideo() $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); - $labels = $annotations->pluck('labels'); + $annotations = collect(collect($yieldAnnotations())->toArray()); $this->assertTrue($annotations->contains('points', [[20, 30, 40]])); - $this->assertFalse($this->isPresent($al1, $labels)); - $this->assertTrue($this->isPresent($al2, $labels)); + $this->assertFalse($annotations->contains('labels', [$al1->toArray()])); + $this->assertTrue($annotations->contains('labels', [$al2->toArray()])); } public function testGetVolumeFileAnnotationsHideBothImage() @@ -296,12 +294,11 @@ public function testGetVolumeFileAnnotationsHideBothImage() $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); - $labels = $annotations->pluck('labels'); + $annotations = collect(collect($yieldAnnotations())->toArray()); $this->assertTrue($annotations->contains('points', [40, 50, 60])); - $this->assertTrue($this->isPresent($al1, $labels)); - $this->assertFalse($this->isPresent($al2, $labels)); + $this->assertTrue($annotations->contains('labels', [$al1->toArray()])); + $this->assertFalse($annotations->contains('labels', [$al2->toArray()])); } public function testGetVolumeFileAnnotationsHideBothVideo() @@ -337,12 +334,11 @@ public function testGetVolumeFileAnnotationsHideBothVideo() $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); - $labels = $annotations->pluck('labels'); + $annotations = collect(collect($yieldAnnotations())->toArray()); $this->assertTrue($annotations->contains('points', [[40, 50, 60]])); - $this->assertTrue($this->isPresent($al1, $labels)); - $this->assertFalse($this->isPresent($al2, $labels)); + $this->assertTrue($annotations->contains('labels', [$al1->toArray()])); + $this->assertFalse($annotations->contains('labels', [$al2->toArray()])); } public function testGetVolumeFileAnnotationsHideNothingImage() @@ -377,12 +373,13 @@ public function testGetVolumeFileAnnotationsHideNothingImage() $yieldAnnotations = $session->getVolumeFileAnnotations($image, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); - $labels = $annotations->pluck('labels'); + $annotations = collect(collect($yieldAnnotations())->toArray()); $this->assertTrue($annotations->contains('points', [40, 50, 60])); - $this->assertTrue($this->isPresent($al1, $labels)); - $this->assertTrue($this->isPresent($al2, $labels)); + $this->assertTrue($annotations->contains('labels', [ + $al1->toArray(), + $al2->toArray(), + ])); } public function testGetVolumeFileAnnotationsHideNothingVideo() @@ -417,12 +414,13 @@ public function testGetVolumeFileAnnotationsHideNothingVideo() $yieldAnnotations = $session->getVolumeFileAnnotations($video, $ownUser); // expand the models in the collection so we can make assertions - $annotations = collect($yieldAnnotations()); - $labels = $annotations->pluck('labels'); + $annotations = collect(collect($yieldAnnotations())->toArray()); $this->assertTrue($annotations->contains('points', [[40, 50, 60]])); - $this->assertTrue($this->isPresent($al1, $labels)); - $this->assertTrue($this->isPresent($al2, $labels)); + $this->assertTrue($annotations->contains('labels', [ + $al1->toArray(), + $al2->toArray(), + ])); } public function testAllowsAccessImageAnnotation() @@ -734,11 +732,4 @@ public function testAnnotationsVideo() $this->assertFalse($session->annotations()->where('id', $a3->id)->exists()); $this->assertFalse($session->annotations()->where('id', $a4->id)->exists()); } - - private function isPresent($needle, $haystack) - { - $needle = collect($needle)->sortKeys(); - $haystack = $haystack->flatten()->map(fn ($l) => collect($l)->sortKeys()); - return $haystack->contains($needle); - } }