diff --git a/app/Actions/Album/Delete.php b/app/Actions/Album/Delete.php index 8181e468610..4c84d6354ee 100644 --- a/app/Actions/Album/Delete.php +++ b/app/Actions/Album/Delete.php @@ -87,7 +87,7 @@ public function do(array $album_ids): FileDeleter /** @var Collection $albums */ /** @phpstan-ignore varTag.type (False positive, NestedSetCollection requires Eloquent Collection) */ $albums = Album::query() - ->without(['cover', 'thumb']) + ->without(['cover', 'thumb', 'min_privilege_cover', 'max_privilege_cover']) ->select(['id', 'parent_id', '_lft', '_rgt', 'track_short_path']) ->findMany($album_ids); @@ -100,7 +100,7 @@ public function do(array $album_ids): FileDeleter /** @var Album $album */ foreach ($albums as $album) { // Collect all (aka recursive) sub-albums in each album - $sub_albums = $album->descendants()->getQuery()->without(['cover', 'thumb'])->select(['id', 'track_short_path'])->get(); + $sub_albums = $album->descendants()->getQuery()->without(['cover', 'thumb', 'min_privilege_cover', 'max_privilege_cover'])->select(['id', 'track_short_path'])->get(); $recursive_album_ids = array_merge($recursive_album_ids, $sub_albums->pluck('id')->all()); $recursive_album_tracks = $recursive_album_tracks->merge($sub_albums->pluck('track_short_path')); } diff --git a/app/Actions/Albums/Flow.php b/app/Actions/Albums/Flow.php index a6c09ea01aa..2aebe86964f 100644 --- a/app/Actions/Albums/Flow.php +++ b/app/Actions/Albums/Flow.php @@ -112,7 +112,13 @@ private function getQuery(Album|null $base, bool $with_relations): AlbumBuilder $base_query = Album::query(); if ($with_relations) { - $base_query->with(['cover', 'cover.size_variants', 'statistics', 'photos', 'photos.statistics', 'photos.size_variants', 'photos.palette', 'photos.tags', 'photos.rating']); + $base_query->with([ + 'cover', 'cover.size_variants', + 'max_privilege_cover', 'max_privilege_cover.size_variants', + 'min_privilege_cover', 'min_privilege_cover.size_variants', + 'statistics', + 'photos', + 'photos.statistics', 'photos.size_variants', 'photos.palette', 'photos.tags', 'photos.rating']); } // Only join what we need for ordering. diff --git a/app/Actions/User/Notify.php b/app/Actions/User/Notify.php index a5b80ef4f2f..5d01f7f591f 100644 --- a/app/Actions/User/Notify.php +++ b/app/Actions/User/Notify.php @@ -46,7 +46,7 @@ public function do(Photo $photo): void // Admin user is always notified $users = User::query()->where('may_administrate', '=', true)->get(); - $albums = Album::query()->without(['thumbs', 'statistics', 'cover'])->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'album.id') + $albums = Album::query()->without(['thumbs', 'statistics', 'cover', 'min_privilege_cover', 'max_privilege_cover'])->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'album.id') ->where(PA::PHOTO_ID, '=', $photo->id) ->get(); diff --git a/app/Models/Album.php b/app/Models/Album.php index c7d6b7d5ef6..fb372785db1 100644 --- a/app/Models/Album.php +++ b/app/Models/Album.php @@ -54,7 +54,9 @@ * @property Carbon|null $max_taken_at Maximum taken_at timestamp of all photos in album and descendants. * @property Carbon|null $min_taken_at Minimum taken_at timestamp of all photos in album and descendants. * @property string|null $auto_cover_id_max_privilege Automatically selected cover photo ID (admin/owner view). + * @property Photo|null $max_privilege_cover * @property string|null $auto_cover_id_least_privilege Automatically selected cover photo ID (most restrictive view). + * @property Photo|null $min_privilege_cover * @property LicenseType $license * @property string|null $cover_id * @property Photo|null $cover @@ -201,7 +203,12 @@ class Album extends BaseAlbum implements Node /** * The relationships that should always be eagerly loaded by default. */ - protected $with = ['cover', 'cover.size_variants', 'thumb']; + protected $with = [ + 'cover', 'cover.size_variants', + 'min_privilege_cover', 'min_privilege_cover.size_variants', + 'max_privilege_cover', 'max_privilege_cover.size_variants', + 'thumb', + ]; /** * Return the relationship between this album and photos which are @@ -265,6 +272,26 @@ public function cover(): HasOne return $this->hasOne(Photo::class, 'id', 'cover_id'); } + /** + * Return the relationship between an album and its min-privilege cover. + * + * @return HasOne + */ + public function min_privilege_cover(): HasOne + { + return $this->hasOne(Photo::class, 'id', 'auto_cover_id_least_privilege'); + } + + /** + * Return the relationship between an album and its max-privilege cover. + * + * @return HasOne + */ + public function max_privilege_cover(): HasOne + { + return $this->hasOne(Photo::class, 'id', 'auto_cover_id_max_privilege'); + } + /** * Return the relationship between an album and its header. * diff --git a/app/Relations/HasAlbumThumb.php b/app/Relations/HasAlbumThumb.php index 176fe1b0b1e..f608be42eea 100644 --- a/app/Relations/HasAlbumThumb.php +++ b/app/Relations/HasAlbumThumb.php @@ -20,7 +20,6 @@ use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Support\Facades\Auth; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Gate; /** @@ -65,22 +64,16 @@ protected function getRelationQuery(): FixedQueryBuilder } /** - * Select the appropriate cover ID based on user privileges. - * - * Priority: - * 1. Explicit cover_id (if set) - * 2. auto_cover_id_max_privilege (if admin or owns album/ancestor) - * 3. auto_cover_id_least_privilege (public view) + * Determine the cover type to use for the album. * * @param Album $album * - * @return string|null + * @return string */ - protected function selectCoverIdForAlbum(Album $album): ?string + protected function getCoverTypeForAlbum(Album $album): string { - // Priority 1: Explicit cover if ($album->cover_id !== null) { - return $album->cover_id; + return 'cover_id'; } /** @var ?User $user */ @@ -88,11 +81,33 @@ protected function selectCoverIdForAlbum(Album $album): ?string // Priority 2: Max-privilege cover for admin or owner if ($user?->may_administrate === true || $album->owner_id === $user?->id) { - return $album->auto_cover_id_max_privilege; + return 'auto_cover_id_max_privilege'; } // Priority 3: Least-privilege cover for public - return $album->auto_cover_id_least_privilege; + return 'auto_cover_id_least_privilege'; + } + + /** + * Select the appropriate cover ID based on user privileges. + * + * Priority: + * 1. Explicit cover_id (if set) + * 2. auto_cover_id_max_privilege (if admin or owns album/ancestor) + * 3. auto_cover_id_least_privilege (public view) + * + * @param Album $album + * + * @return string|null + */ + protected function selectCoverIdForAlbum(Album $album): ?string + { + return match ($this->getCoverTypeForAlbum($album)) { + 'cover_id' => $album->cover_id, + 'auto_cover_id_max_privilege' => $album->auto_cover_id_max_privilege, + 'auto_cover_id_least_privilege' => $album->auto_cover_id_least_privilege, + default => null, + }; } /** @@ -130,42 +145,17 @@ public function addConstraints(): void } /** - * Builds a query to eagerly load the thumbnails of a sequence of albums. - * - * Now uses pre-computed cover IDs (auto_cover_id_max_privilege and - * auto_cover_id_least_privilege) instead of expensive runtime queries. + * We do not eager load any covers. + * This relation is only meaningful for single albums. + * In case of multiple album we use the preloaded values from + * `cover_id`, `auto_cover_id_max_privilege`, and `auto_cover_id_least_privilege`. * * @param array $models */ public function addEagerConstraints(array $models): void { - // Build mapping of album_id => cover_id for each album - $album_to_cover = []; - foreach ($models as $album) { - $cover_id = $this->selectCoverIdForAlbum($album); - if ($cover_id !== null) { - $album_to_cover[$album->id] = $cover_id; - } - } - - if (count($album_to_cover) === 0) { - // No covers to load - make query return empty result - $this->getRelationQuery()->whereRaw('1 = 0'); - - return; - } - - // Select photos with their album association - $this->getRelationQuery() - ->select([ - 'photos.id as id', - 'photos.type as type', - DB::raw('CASE ' . - collect($album_to_cover)->map(fn ($cover_id, $album_id) => "WHEN photos.id = '$cover_id' THEN '$album_id'" - )->implode(' ') . - ' END as covered_album_id'), - ]) - ->whereIn('photos.id', array_values($album_to_cover)); + // No covers to load - make query return empty result + $this->getRelationQuery()->whereRaw('1 = 0'); } /** @@ -194,18 +184,17 @@ public function initRelation(array $models, $relation): array */ public function match(array $models, Collection $results, $relation): array { - $dictionary = $results->mapToDictionary(function ($result) { - /** @var Photo&object{covered_album_id: string} $result */ - return [$result->covered_album_id => $result]; - })->all(); - - // Match photos to albums using the covered_album_id /** @var Album $album */ foreach ($models as $album) { - $album_id = $album->id; - if (isset($dictionary[$album_id])) { - $cover = reset($dictionary[$album_id]); - $album->setRelation($relation, Thumb::createFromPhoto($cover)); + $cover_type = $this->getCoverTypeForAlbum($album); + if ($cover_type === 'cover_id' && $album->cover_id !== null) { + // We do not need to do anything here, because we already have the cover + // loaded via the `cover` relation of `Album`. + $album->setRelation($relation, Thumb::createFromPhoto($album->cover)); + } elseif ($cover_type === 'auto_cover_id_max_privilege' && $album->auto_cover_id_max_privilege !== null) { + $album->setRelation($relation, Thumb::createFromPhoto($album->max_privilege_cover)); + } elseif ($cover_type === 'auto_cover_id_least_privilege' && $album->auto_cover_id_least_privilege !== null) { + $album->setRelation($relation, Thumb::createFromPhoto($album->min_privilege_cover)); } else { $album->setRelation($relation, null); } @@ -226,21 +215,19 @@ public function getResults(): ?Thumb // is always eagerly loaded with its cover and hence, we already // have it. // See {@link Album::with} - $cover_id = $this->selectCoverIdForAlbum($album); - if ($cover_id === $album->cover_id) { + $cover_type = $this->getCoverTypeForAlbum($album); + if ($cover_type === 'cover_id' && $album->cover_id !== null) { + // We do not need to do anything here, because we already have the cover + // loaded via the `cover` relation of `Album`. return Thumb::createFromPhoto($album->cover); - } - - if ($cover_id !== null) { - // Use pre-computed cover ID (explicit, max-privilege, or least-privilege) - $photo = Photo::query()->with(['size_variants' => (fn ($r) => Thumb::sizeVariantsFilter($r))])->find($cover_id); - - return Thumb::createFromPhoto($photo); + } elseif ($cover_type === 'auto_cover_id_max_privilege' && $album->auto_cover_id_max_privilege !== null) { + return Thumb::createFromPhoto($album->max_privilege_cover); + } elseif ($cover_type === 'auto_cover_id_least_privilege' && $album->auto_cover_id_least_privilege !== null) { + return Thumb::createFromPhoto($album->min_privilege_cover); } else { - // Fallback to legacy query if no cover available return Thumb::createFromQueryable( $this->getRelationQuery(), - $this->sorting, + $this->sorting ); } } diff --git a/tests/Unit/CoverageTest.php b/tests/Unit/CoverageTest.php index a75dc61a890..d3f70b01d35 100644 --- a/tests/Unit/CoverageTest.php +++ b/tests/Unit/CoverageTest.php @@ -31,6 +31,9 @@ use App\Factories\AlbumFactory; use App\Image\Files\ProcessableJobFile; use App\Jobs\ProcessImageJob; +use App\Models\Album; +use App\Models\User; +use App\Relations\HasAlbumThumb; use App\SmartAlbums\UnsortedAlbum; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Support\Facades\Auth; @@ -187,4 +190,172 @@ public function testJobFailing(): void $user->delete(); self::assertTrue(true); } + + public function testHasAlbumThumbCoverTypeForExplicitCover(): void + { + // Mock album with explicit cover_id + $album = \Mockery::mock(Album::class)->makePartial(); + $album->cover_id = 'explicit-cover-id'; + $album->auto_cover_id_max_privilege = 'max-priv-id'; + $album->auto_cover_id_least_privilege = 'least-priv-id'; + + $relation = new HasAlbumThumb($album); + $method = new \ReflectionMethod(HasAlbumThumb::class, 'getCoverTypeForAlbum'); + + $cover_type = $method->invoke($relation, $album); + self::assertEquals('cover_id', $cover_type); + } + + public function testHasAlbumThumbCoverTypeForAdminUser(): void + { + // Mock admin user + $admin = \Mockery::mock(User::class)->makePartial(); + $admin->id = 1; + $admin->may_administrate = true; + + Auth::shouldReceive('user')->andReturn($admin); + + // Mock album without explicit cover + $album = \Mockery::mock(Album::class)->makePartial(); + $album->cover_id = null; + $album->owner_id = 999; + $album->auto_cover_id_max_privilege = 'max-priv-id'; + $album->auto_cover_id_least_privilege = 'least-priv-id'; + + $relation = new HasAlbumThumb($album); + $method = new \ReflectionMethod(HasAlbumThumb::class, 'getCoverTypeForAlbum'); + + $cover_type = $method->invoke($relation, $album); + self::assertEquals('auto_cover_id_max_privilege', $cover_type); + } + + public function testHasAlbumThumbCoverTypeForOwner(): void + { + // Mock owner user + $owner = \Mockery::mock(User::class)->makePartial(); + $owner->id = 42; + $owner->may_administrate = false; + + Auth::shouldReceive('user')->andReturn($owner); + + // Mock album owned by user + $album = \Mockery::mock(Album::class)->makePartial(); + $album->cover_id = null; + $album->owner_id = 42; + $album->auto_cover_id_max_privilege = 'max-priv-id'; + $album->auto_cover_id_least_privilege = 'least-priv-id'; + + $relation = new HasAlbumThumb($album); + $method = new \ReflectionMethod(HasAlbumThumb::class, 'getCoverTypeForAlbum'); + + $cover_type = $method->invoke($relation, $album); + self::assertEquals('auto_cover_id_max_privilege', $cover_type); + } + + public function testHasAlbumThumbCoverTypeForPublicUser(): void + { + // Mock no authenticated user (public view) + Auth::shouldReceive('user')->andReturn(null); + + // Mock album without explicit cover + $album = \Mockery::mock(Album::class)->makePartial(); + $album->cover_id = null; + $album->owner_id = 999; + $album->auto_cover_id_max_privilege = 'max-priv-id'; + $album->auto_cover_id_least_privilege = 'least-priv-id'; + + $relation = new HasAlbumThumb($album); + $method = new \ReflectionMethod(HasAlbumThumb::class, 'getCoverTypeForAlbum'); + + $cover_type = $method->invoke($relation, $album); + self::assertEquals('auto_cover_id_least_privilege', $cover_type); + } + + public function testHasAlbumThumbSelectCoverIdWithExplicitCover(): void + { + $album = \Mockery::mock(Album::class)->makePartial(); + $album->cover_id = 'explicit-cover-id'; + $album->auto_cover_id_max_privilege = 'max-priv-id'; + $album->auto_cover_id_least_privilege = 'least-priv-id'; + + $relation = new HasAlbumThumb($album); + $method = new \ReflectionMethod(HasAlbumThumb::class, 'selectCoverIdForAlbum'); + + $selected_id = $method->invoke($relation, $album); + self::assertEquals('explicit-cover-id', $selected_id); + } + + public function testHasAlbumThumbSelectCoverIdWithMaxPrivilege(): void + { + // Mock admin user + $admin = \Mockery::mock(User::class)->makePartial(); + $admin->id = 1; + $admin->may_administrate = true; + + Auth::shouldReceive('user')->andReturn($admin); + + $album = \Mockery::mock(Album::class)->makePartial(); + $album->cover_id = null; + $album->owner_id = 999; + $album->auto_cover_id_max_privilege = 'max-priv-id'; + $album->auto_cover_id_least_privilege = 'least-priv-id'; + + $relation = new HasAlbumThumb($album); + $method = new \ReflectionMethod(HasAlbumThumb::class, 'selectCoverIdForAlbum'); + + $selected_id = $method->invoke($relation, $album); + self::assertEquals('max-priv-id', $selected_id); + } + + public function testHasAlbumThumbSelectCoverIdWithLeastPrivilege(): void + { + // Mock no authenticated user (public view) + Auth::shouldReceive('user')->andReturn(null); + + $album = \Mockery::mock(Album::class)->makePartial(); + $album->cover_id = null; + $album->owner_id = 999; + $album->auto_cover_id_max_privilege = 'max-priv-id'; + $album->auto_cover_id_least_privilege = 'least-priv-id'; + + $relation = new HasAlbumThumb($album); + $method = new \ReflectionMethod(HasAlbumThumb::class, 'selectCoverIdForAlbum'); + + $selected_id = $method->invoke($relation, $album); + self::assertEquals('least-priv-id', $selected_id); + } + + public function testHasAlbumThumbSelectCoverIdReturnsNull(): void + { + // Mock no authenticated user (public view) + Auth::shouldReceive('user')->andReturn(null); + + // Test the case where no cover IDs are available + // We can't instantiate HasAlbumThumb when all covers are null because + // it triggers the fallback query in addConstraints() which requires DB. + // Instead, we test the logic by verifying getCoverTypeForAlbum returns + // the expected type and that selectCoverIdForAlbum would return null. + + $album = \Mockery::mock(Album::class)->makePartial(); + $album->cover_id = null; + $album->auto_cover_id_max_privilege = null; + $album->auto_cover_id_least_privilege = null; + $album->is_nsfw = false; + $album->shouldReceive('getAttribute') + ->with('owner_id') + ->andReturn(999); + + // Use a different album with non-null cover for the parent so + // HasAlbumThumb can be instantiated without triggering DB query + $parentAlbum = \Mockery::mock(Album::class)->makePartial(); + $parentAlbum->cover_id = 'some-cover-id'; // Non-null to avoid fallback + $parentAlbum->is_nsfw = false; + + $relation = new HasAlbumThumb($parentAlbum); + $method = new \ReflectionMethod(HasAlbumThumb::class, 'selectCoverIdForAlbum'); + + // Test with the album that has null covers + $selected_id = $method->invoke($relation, $album); + self::assertNull($selected_id); + } }