diff --git a/app/Actions/Album/Delete.php b/app/Actions/Album/Delete.php index c02f26bd0cb..899084fc19f 100644 --- a/app/Actions/Album/Delete.php +++ b/app/Actions/Album/Delete.php @@ -23,11 +23,9 @@ use App\Models\BaseAlbumImpl; use App\Models\Statistics; use App\Models\TagAlbum; -use App\SmartAlbums\UnsortedAlbum; use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Database\Query\Builder as BaseBuilder; -use Illuminate\Support\Facades\Auth; use Safe\Exceptions\ArrayException; /** @@ -73,18 +71,6 @@ class Delete public function do(array $album_ids): FileDeleter { try { - $unsorted_photo_ids = []; - - // Among the smart albums, the unsorted album is special, - // because it provides deletion of photos - if (in_array(UnsortedAlbum::ID, $album_ids, true)) { - $query = UnsortedAlbum::getInstance()->photos(); - if (Auth::user()?->may_administrate !== true) { - $query->where('owner_id', '=', Auth::id() ?? throw new UnauthenticatedException()); - } - $unsorted_photo_ids = $query->pluck('id')->all(); - } - // Only regular albums are owners of photos, so we only need to // find all photos in those and their descendants // Only load necessary attributes for tree; in particular avoid @@ -111,7 +97,7 @@ public function do(array $album_ids): FileDeleter // Delete the photos from DB and obtain the list of files which need // to be deleted later - $file_deleter = (new PhotoDelete())->do($unsorted_photo_ids, $recursive_album_ids); + $file_deleter = (new PhotoDelete())->do([], null, $recursive_album_ids); $file_deleter->addFiles($recursive_album_tracks, StorageDiskType::LOCAL->value); // Remove the sub-forest spanned by the regular albums diff --git a/app/Actions/Album/Merge.php b/app/Actions/Album/Merge.php index d9d7c4bc84b..a33cddd4611 100644 --- a/app/Actions/Album/Merge.php +++ b/app/Actions/Album/Merge.php @@ -8,12 +8,13 @@ namespace App\Actions\Album; +use App\Constants\PhotoAlbum as PA; use App\Exceptions\Internal\QueryBuilderException; use App\Exceptions\ModelDBException; use App\Models\Album; -use App\Models\Photo; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\DB; class Merge { @@ -27,10 +28,28 @@ class Merge */ public function do(Album $target_album, Collection $albums): void { - // Merge photos of source albums into target - Photo::query() - ->whereIn('album_id', $albums->pluck('id')) - ->update(['album_id' => $target_album->id]); + $origin_ids = $albums->pluck('id')->all(); + + // Select all photos ids of the source albums + $photos_ids = DB::table(PA::PHOTO_ALBUM) + ->whereIn(PA::ALBUM_ID, $origin_ids) + ->pluck(PA::PHOTO_ID)->all(); + + // Delete the existing links at destination (avoid duplicates key contraint) + DB::table(PA::PHOTO_ALBUM) + ->whereIn(PA::PHOTO_ID, $photos_ids) + ->where(PA::ALBUM_ID, '=', $target_album->id) + ->delete(); + + // Insert the new links + DB::table(PA::PHOTO_ALBUM) + ->insert(array_map(fn (string $id) => ['photo_id' => $id, 'album_id' => $target_album->id], $photos_ids)); + + // Delete the existing links from the origins + DB::table(PA::PHOTO_ALBUM) + ->whereIn(PA::PHOTO_ID, $photos_ids) + ->whereIn(PA::ALBUM_ID, $origin_ids) + ->delete(); // Merge sub-albums of source albums into target /** @var Album $album */ diff --git a/app/Actions/Album/PositionData.php b/app/Actions/Album/PositionData.php index c9ca169b087..3ea1ab778c9 100644 --- a/app/Actions/Album/PositionData.php +++ b/app/Actions/Album/PositionData.php @@ -12,7 +12,6 @@ use App\Enum\SizeVariantType; use App\Http\Resources\Collections\PositionDataResource; use App\Models\Album; -use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; class PositionData @@ -25,13 +24,6 @@ public function get(AbstractAlbum $album, bool $include_sub_albums = false): Pos $photo_relation ->with([ - 'album' => function (BelongsTo $b): void { - // The album is required for photos to properly - // determine access and visibility rights; but we - // don't need to determine the cover and thumbnail for - // each album - $b->without(['cover', 'thumb']); - }, 'statistics', 'size_variants' => function (HasMany $r): void { // The web GUI only uses the small and thumb size @@ -47,6 +39,6 @@ public function get(AbstractAlbum $album, bool $include_sub_albums = false): Pos ->whereNotNull('latitude') ->whereNotNull('longitude'); - return new PositionDataResource($album->get_id(), $album->get_title(), $photo_relation->get(), $album instanceof Album ? $album->track_url : null); + return new PositionDataResource($album, $photo_relation->get(), $album instanceof Album ? $album->track_url : null); } } diff --git a/app/Actions/Albums/PositionData.php b/app/Actions/Albums/PositionData.php index d33eadeec4f..512fae5fd52 100644 --- a/app/Actions/Albums/PositionData.php +++ b/app/Actions/Albums/PositionData.php @@ -38,13 +38,6 @@ public function do(): PositionDataResource $photo_query = $this->photo_query_policy->applySearchabilityFilter( query: Photo::query() ->with([ - 'album' => function ($b): void { - // The album is required for photos to properly - // determine access and visibility rights; but we - // don't need to determine the cover and thumbnail for - // each album - $b->without(['cover', 'thumb']); - }, 'statistics', 'size_variants' => function ($r): void { // The web GUI only uses the small and thumb size @@ -63,6 +56,6 @@ public function do(): PositionDataResource include_nsfw: !Configs::getValueAsBool('hide_nsfw_in_map') ); - return new PositionDataResource(null, null, $photo_query->get(), null); + return new PositionDataResource(null, $photo_query->get(), null); } } diff --git a/app/Actions/Diagnostics/Pipes/Checks/DBIntegrityCheck.php b/app/Actions/Diagnostics/Pipes/Checks/DBIntegrityCheck.php index 91b957e0b1e..8df24b1d796 100644 --- a/app/Actions/Diagnostics/Pipes/Checks/DBIntegrityCheck.php +++ b/app/Actions/Diagnostics/Pipes/Checks/DBIntegrityCheck.php @@ -33,14 +33,14 @@ public function handle(array &$data, \Closure $next): array $sub_join = DB::table('size_variants')->where('size_variants.type', '=', 0); $photos = Photo::query() - ->with(['album']) - ->select(['photos.id', 'title', 'album_id']) + ->with(['albums']) + ->select(['photos.id', 'title']) ->joinSub($sub_join, 'size_variants', 'size_variants.photo_id', '=', 'photos.id', 'left') ->whereNull('size_variants.id') ->get(); foreach ($photos as $photo) { - $data[] = DiagnosticData::error('Photo without Original found -- ' . $photo->title . ' in ' . ($photo->album?->title ?? __('gallery.smart_album.unsorted')), self::class); + $data[] = DiagnosticData::error('Photo without Original found -- ' . $photo->title . ' in ' . ($photo->albums?->first()?->title ?? __('gallery.smart_album.unsorted')), self::class); } return $next($data); diff --git a/app/Actions/Photo/Create.php b/app/Actions/Photo/Create.php index 81f0078c54a..0a8692217bc 100644 --- a/app/Actions/Photo/Create.php +++ b/app/Actions/Photo/Create.php @@ -139,10 +139,9 @@ private function handleDuplicate(InitDTO $init_dto): Photo $pipes[] = Duplicate\SaveIfDirty::class; } $pipes[] = Duplicate\ThrowSkipDuplicate::class; - $pipes[] = Duplicate\ReplicateAsPhoto::class; $pipes[] = Shared\SetStarred::class; - $pipes[] = Shared\SetParentAndOwnership::class; $pipes[] = Shared\Save::class; + $pipes[] = Shared\SetParent::class; $pipes[] = Shared\SaveStatistics::class; $pipes[] = Shared\NotifyAlbums::class; @@ -167,7 +166,7 @@ private function handleStandalone(InitDTO $init_dto): Photo Standalone\InitNamingStrategy::class, Shared\HydrateMetadata::class, Shared\SetStarred::class, - Shared\SetParentAndOwnership::class, + Shared\SetOwnership::class, Standalone\SetOriginalChecksum::class, Standalone\FetchSourceImage::class, Standalone\ExtractGoogleMotionPictures::class, @@ -175,6 +174,7 @@ private function handleStandalone(InitDTO $init_dto): Photo Standalone\PlaceGoogleMotionVideo::class, Standalone\SetChecksum::class, Shared\Save::class, + Shared\SetParent::class, Shared\SaveStatistics::class, Standalone\CreateOriginalSizeVariant::class, Standalone\CreateSizeVariants::class, @@ -258,7 +258,7 @@ private function handlePhotoLivePartner(InitDTO $init_dto): Photo Standalone\InitNamingStrategy::class, Shared\HydrateMetadata::class, Shared\SetStarred::class, - Shared\SetParentAndOwnership::class, + Shared\SetOwnership::class, Standalone\SetOriginalChecksum::class, Standalone\FetchSourceImage::class, Standalone\ExtractGoogleMotionPictures::class, @@ -266,6 +266,7 @@ private function handlePhotoLivePartner(InitDTO $init_dto): Photo Standalone\PlaceGoogleMotionVideo::class, Standalone\SetChecksum::class, Shared\Save::class, + Shared\SetParent::class, Standalone\CreateOriginalSizeVariant::class, Standalone\CreateSizeVariants::class, Standalone\EncodePlaceholder::class, diff --git a/app/Actions/Photo/Delete.php b/app/Actions/Photo/Delete.php index 6724339e1a0..ed06677ba46 100644 --- a/app/Actions/Photo/Delete.php +++ b/app/Actions/Photo/Delete.php @@ -8,8 +8,10 @@ namespace App\Actions\Photo; +use App\Constants\PhotoAlbum as PA; use App\Enum\SizeVariantType; use App\Exceptions\Internal\LycheeAssertionError; +use App\Exceptions\Internal\LycheeLogicException; use App\Exceptions\Internal\QueryBuilderException; use App\Exceptions\ModelDBException; use App\Image\FileDeleter; @@ -20,6 +22,7 @@ use App\Models\Statistics; use Illuminate\Database\Query\Builder as BaseBuilder; use Illuminate\Database\Query\JoinClause; +use Illuminate\Support\Facades\DB; /** * Deletes the photos with the designated IDs **efficiently**. @@ -69,21 +72,43 @@ public function __construct() * Both parameters can be used simultaneously and result in a merged * deletion of the joined set of photos. * - * @param string[] $photo_ids the photo IDs - * @param string[] $album_ids the album IDs + * Note that this methods does not assume the recursion within albums ids. + * If albums_ids has children, photos within those children will not be deleted. + * + * @param string[] $photo_ids the photo IDs + * @param string|null $from_id the ID of the album from which the photos are deleted + * @param string[] $album_ids the album IDs * * @return FileDeleter contains the collected files which became obsolete * * @throws ModelDBException */ - public function do(array $photo_ids, array $album_ids = []): FileDeleter + public function do(array $photo_ids, string|null $from_id, array $album_ids = []): FileDeleter { - // TODO: replace this with pipelines, This is typically the kind of pattern. + $this->validateArguments($photo_ids, $from_id, $album_ids); + + // First find out which photos do not have an album. + // Those will be deleted. + $unsorted_photo_ids = $this->collectUnsortedPhotos($photo_ids); + $photo_ids = $this->collectPhotosInAlbums($photo_ids); + + if (count($photo_ids) > 0) { + // We delete the photos which are in the from and listed in albums. + DB::table(PA::PHOTO_ALBUM)->whereIn(PA::PHOTO_ID, $photo_ids)->where(PA::ALBUM_ID, '=', $from_id)->delete(); + } else { + $photo_ids = $this->collectPhotosInAlbumsByAlbumID($album_ids); + // We delete the photos which are in the from and listed in albums. + DB::table(PA::PHOTO_ALBUM)->whereIn(PA::ALBUM_ID, $album_ids)->delete(); + } + + // Now that the relation is destroyed, we need to figure out which photos really need to be deleted: + // Those are the ones that were previously in albums but not anymore. + $photo_ids = $this->collectUnsortedPhotos($photo_ids); + $photo_ids = array_merge($photo_ids, $unsorted_photo_ids); + try { $this->collectSizeVariantPathsByPhotoID($photo_ids); - $this->collectSizeVariantPathsByAlbumID($album_ids); $this->collectLivePhotoPathsByPhotoID($photo_ids); - $this->collectLivePhotoPathsByAlbumID($album_ids); $this->deleteDBRecords($photo_ids, $album_ids); // @codeCoverageIgnoreStart } catch (QueryBuilderException $e) { @@ -96,44 +121,76 @@ public function do(array $photo_ids, array $album_ids = []): FileDeleter } /** - * Collects all short paths of size variants which shall be deleted from - * disk. + * We make sure that only the valid code path are used to ensure the integrity + * of the database. * - * Size variants which belong to a photo which has a duplicate that is - * not going to be deleted are skipped. - * - * @param array $photo_ids the photo IDs + * @param array $photo_ids + * @param string|null $from_id + * @param array $album_ids * * @return void * - * @throws QueryBuilderException + * @throws LycheeLogicException */ - private function collectSizeVariantPathsByPhotoID(array $photo_ids): void + private function validateArguments(array $photo_ids, string|null $from_id, array $album_ids): void { - try { - if (count($photo_ids) === 0) { - return; - } + match (true) { + count($photo_ids) !== 0 && count($album_ids) !== 0 => throw new LycheeLogicException('Only one of the arguments [$photo_ids, $album_ids] must be set.'), + count($photo_ids) !== 0 && $from_id === null => throw new LycheeLogicException('The $from_id must be provided with the $photo_ids.'), + count($album_ids) !== 0 && $from_id !== null => throw new LycheeLogicException('The $from_id must not be provided with the $album_ids.'), + default => null, // do nothing :) + }; + } - // Maybe consider doing multiple queries for the different storage types. - $size_variants = SizeVariant::query() - ->from('size_variants as sv') - ->select(['sv.short_path', 'sv.storage_disk']) - ->join('photos as p', 'p.id', '=', 'sv.photo_id') - ->leftJoin('photos as dup', function (JoinClause $join) use ($photo_ids): void { - $join - ->on('dup.checksum', '=', 'p.checksum') - ->whereNotIn('dup.id', $photo_ids); - }) - ->whereIn('p.id', $photo_ids) - ->whereNull('dup.id') - ->get(); - $this->fileDeleter->addSizeVariants($size_variants); - // @codeCoverageIgnoreStart - } catch (\InvalidArgumentException $e) { - throw LycheeAssertionError::createFromUnexpectedException($e); + /** + * We select all photos which are not in an album and in the preselection. + * + * @param string[] $photo_ids + * + * @return string[] Photo IDs + */ + private function collectUnsortedPhotos(array $photo_ids): array + { + if (count($photo_ids) === 0) { + return []; } - // @codeCoverageIgnoreEnd + + return Photo::query()->leftJoin(PA::PHOTO_ALBUM, PA::PHOTO_ID, '=', 'photos.id') + ->whereIn('photos.id', $photo_ids) + ->whereNull(PA::ALBUM_ID) + ->select(['photos.id'])->toBase()->pluck('id')->all(); + } + + /** + * We select all the photos which are in an album and in the preselection. + * + * @param string[] $photo_ids + * + * @return string[] photo IDs + */ + private function collectPhotosInAlbums(array $photo_ids): array + { + if (count($photo_ids) === 0) { + return []; + } + + return DB::table(PA::PHOTO_ALBUM)->whereIn(PA::PHOTO_ID, $photo_ids)->select([PA::PHOTO_ID])->pluck('photo_id')->all(); + } + + /** + * We select all the photos which are in a list of albums. + * + * @param string[] $album_ids + * + * @return string[] photo IDs + */ + private function collectPhotosInAlbumsByAlbumID(array $album_ids): array + { + if (count($album_ids) === 0) { + return []; + } + + return DB::table(PA::PHOTO_ALBUM)->whereIn(PA::ALBUM_ID, $album_ids)->select([PA::PHOTO_ID])->pluck('photo_id')->all(); } /** @@ -143,16 +200,16 @@ private function collectSizeVariantPathsByPhotoID(array $photo_ids): void * Size variants which belong to a photo which has a duplicate that is * not going to be deleted are skipped. * - * @param array $album_ids the album IDs + * @param array $photo_ids the photo IDs * * @return void * * @throws QueryBuilderException */ - private function collectSizeVariantPathsByAlbumID(array $album_ids): void + private function collectSizeVariantPathsByPhotoID(array $photo_ids): void { try { - if (count($album_ids) === 0) { + if (count($photo_ids) === 0) { return; } @@ -161,12 +218,12 @@ private function collectSizeVariantPathsByAlbumID(array $album_ids): void ->from('size_variants as sv') ->select(['sv.short_path', 'sv.storage_disk']) ->join('photos as p', 'p.id', '=', 'sv.photo_id') - ->leftJoin('photos as dup', function (JoinClause $join) use ($album_ids): void { + ->leftJoin('photos as dup', function (JoinClause $join) use ($photo_ids): void { $join ->on('dup.checksum', '=', 'p.checksum') - ->whereNotIn('dup.album_id', $album_ids); + ->whereNotIn('dup.id', $photo_ids); }) - ->whereIn('p.album_id', $album_ids) + ->whereIn('p.id', $photo_ids) ->whereNull('dup.id') ->get(); $this->fileDeleter->addSizeVariants($size_variants); @@ -226,55 +283,6 @@ private function collectLivePhotoPathsByPhotoID(array $photo_ids) // @codeCoverageIgnoreEnd } - /** - * Collects all short paths of live photos which shall be deleted from - * disk. - * - * Live photos which have a duplicate that is not going to be deleted are - * skipped. - * - * @param array $album_ids the album IDs - * - * @return void - * - * @throws QueryBuilderException - */ - private function collectLivePhotoPathsByAlbumID(array $album_ids) - { - try { - if (count($album_ids) === 0) { - return; - } - - $live_photo_short_paths = Photo::query() - ->from('photos as p') - ->select(['p.live_photo_short_path', 'sv.storage_disk']) - ->join('size_variants as sv', function (JoinClause $join): void { - $join - ->on('sv.photo_id', '=', 'p.id') - ->where('sv.type', '=', SizeVariantType::ORIGINAL); - }) - ->leftJoin('photos as dup', function (JoinClause $join) use ($album_ids): void { - $join - ->on('dup.live_photo_checksum', '=', 'p.live_photo_checksum') - ->whereNotIn('dup.album_id', $album_ids); - }) - ->whereIn('p.album_id', $album_ids) - ->whereNull('dup.id') - ->whereNotNull('p.live_photo_short_path') - ->get(['p.live_photo_short_path', 'sv.storage_disk']); - - $live_variants_short_paths_grouped = $live_photo_short_paths->groupBy('storage_disk'); - $live_variants_short_paths_grouped->each( - fn ($live_variants_short_paths, $disk) => $this->fileDeleter->addFiles($live_variants_short_paths->map(fn ($lv) => $lv->live_photo_short_path), $disk) - ); - // @codeCoverageIgnoreStart - } catch (\InvalidArgumentException $e) { - throw LycheeAssertionError::createFromUnexpectedException($e); - } - // @codeCoverageIgnoreEnd - } - /** * Deletes the records from DB. * @@ -302,7 +310,8 @@ private function deleteDBRecords(array $photo_ids, array $album_ids): void $query ->from('photos', 'p') ->whereColumn('p.id', '=', 'size_variants.photo_id') - ->whereIn('p.album_id', $album_ids); + ->leftJoin(PA::PHOTO_ALBUM, PA::PHOTO_ID, '=', 'p.id') + ->whereIn(PA::ALBUM_ID, $album_ids); }) ->delete(); } @@ -317,7 +326,8 @@ private function deleteDBRecords(array $photo_ids, array $album_ids): void $query ->from('photos', 'p') ->whereColumn('p.id', '=', 'statistics.photo_id') - ->whereIn('p.album_id', $album_ids); + ->leftJoin(PA::PHOTO_ALBUM, PA::PHOTO_ID, '=', 'p.id') + ->whereIn(PA::ALBUM_ID, $album_ids); }) ->delete(); } @@ -332,16 +342,14 @@ private function deleteDBRecords(array $photo_ids, array $album_ids): void $query ->from('photos', 'p') ->whereColumn('p.id', '=', 'palettes.photo_id') - ->whereIn('p.album_id', $album_ids); + ->leftJoin(PA::PHOTO_ALBUM, PA::PHOTO_ID, '=', 'p.id') + ->whereIn(PA::ALBUM_ID, $album_ids); }) ->delete(); } if (count($photo_ids) !== 0) { Photo::query()->whereIn('id', $photo_ids)->delete(); } - if (count($album_ids) !== 0) { - Photo::query()->whereIn('album_id', $album_ids)->delete(); - } // @codeCoverageIgnoreStart } catch (\InvalidArgumentException $e) { throw LycheeAssertionError::createFromUnexpectedException($e); diff --git a/app/Actions/Photo/Duplicate.php b/app/Actions/Photo/Duplicate.php deleted file mode 100644 index 362cad84366..00000000000 --- a/app/Actions/Photo/Duplicate.php +++ /dev/null @@ -1,45 +0,0 @@ - $photos the source photos - * @param Album|null $album the destination album; `null` means root album - * - * @return Collection the duplicates - * - * @throws ModelDBException - */ - public function do(Collection $photos, ?Album $album): Collection - { - $duplicates = new Collection(); - /** @var Photo $photo */ - foreach ($photos as $photo) { - $duplicate = $photo->replicate(); - $duplicate->album_id = $album?->id; - $duplicate->setRelation('album', $album); - if ($album !== null) { - $duplicate->owner_id = $album->owner_id; - } - $duplicate->save(); - $duplicates->add($duplicate); - } - - return $duplicates; - } -} diff --git a/app/Actions/Photo/DuplicateFinder.php b/app/Actions/Photo/DuplicateFinder.php index b1fa78b9f20..fb36e62085b 100644 --- a/app/Actions/Photo/DuplicateFinder.php +++ b/app/Actions/Photo/DuplicateFinder.php @@ -8,6 +8,7 @@ namespace App\Actions\Photo; +use App\Constants\PhotoAlbum as PA; use App\Exceptions\Internal\LycheeLogicException; use App\Exceptions\Internal\QueryBuilderException; use App\Models\Photo; @@ -77,7 +78,8 @@ private function query( } return Photo::query() - ->join('base_albums', 'base_albums.id', '=', 'photos.album_id') + ->join(PA::PHOTO_ALBUM, PA::PHOTO_ID, '=', 'photos.id') + ->join('base_albums', 'base_albums.id', '=', PA::ALBUM_ID) ->join( 'size_variants', 'size_variants.photo_id', '=', 'photos.id', 'left' ) @@ -104,12 +106,14 @@ private function getDuplicatesIdsQuery( bool $must_have_same_title, ): Builder { return DB::table('photos', 'p1')->select('p1.id') + ->joinSub(DB::table(PA::PHOTO_ALBUM), 'pa1', 'pa1.photo_id', '=', 'p1.id', 'left') ->join( 'photos as p2', fn ($join) => $join->on('p1.id', '<>', 'p2.id') + ->joinSub(DB::table(PA::PHOTO_ALBUM), 'pa2', 'pa2.photo_id', '=', 'p2.id', 'left') ->when($must_have_same_title, fn ($q) => $q->on('p1.title', '=', 'p2.title')) ->when($must_have_same_checksum, fn ($q) => $q->on('p1.checksum', '=', 'p2.checksum')) - ->when($must_be_within_same_album, fn ($q) => $q->on('p1.album_id', '=', 'p2.album_id')) + ->when($must_be_within_same_album, fn ($q) => $q->on('pa1.album_id', '=', 'pa2.album_id')) ); } } \ No newline at end of file diff --git a/app/Actions/Photo/Move.php b/app/Actions/Photo/Move.php deleted file mode 100644 index 5ba4353fe12..00000000000 --- a/app/Actions/Photo/Move.php +++ /dev/null @@ -1,48 +0,0 @@ - $photos the source photos - * @param Album|null $album the destination album; `null` means root album - * - * @return void - * - * @throws ModelDBException - */ - public function do(Collection $photos, ?Album $album): void - { - $notify = new Notify(); - - /** @var Photo $photo */ - foreach ($photos as $photo) { - $photo->album_id = $album?->id; - // Avoid unnecessary DB request, when we access the album of a - // photo later (e.g. when a notification is sent). - $photo->setRelation('album', $album); - if ($album !== null) { - $photo->owner_id = $album->owner_id; - } - $photo->save(); - $notify->do($photo); - } - - Album::query()->whereIn('header_id', $photos->map(fn (Photo $p) => $p->id))->update(['header_id' => null]); - } -} diff --git a/app/Actions/Photo/MoveOrDuplicate.php b/app/Actions/Photo/MoveOrDuplicate.php new file mode 100644 index 00000000000..f8a0240ebfe --- /dev/null +++ b/app/Actions/Photo/MoveOrDuplicate.php @@ -0,0 +1,72 @@ + $photos the source photos + * @param AbstractAlbum $from_album the origin album; `null` means root album + * @param Album $to_album the destination album; `null` means root album + * + * @return void + */ + public function do(Collection $photos, ?AbstractAlbum $from_album, ?Album $to_album): void + { + // Extract the photos Ids. + $photos_ids = $photos->pluck('id')->all(); + + if ($from_album !== null) { + // Delete the existing links. + DB::table(PA::PHOTO_ALBUM) + ->whereIn(PA::PHOTO_ID, $photos_ids) + ->where(PA::ALBUM_ID, '=', $from_album->get_id()) + ->delete(); + } + + if ($to_album !== null) { + // Delete the existing links at destination (avoid duplicates key contraint) + // If $from === to this operation is not needed. + DB::table(PA::PHOTO_ALBUM) + ->whereIn(PA::PHOTO_ID, $photos_ids) + ->where(PA::ALBUM_ID, '=', $to_album->id) + ->delete(); + + // Add the new links. + DB::table(PA::PHOTO_ALBUM)->insert(array_map(fn (string $id) => ['photo_id' => $id, 'album_id' => $to_album->id], $photos_ids)); + } + + // In case of move, we need to remove the header_id of said photos. + if ($from_album !== null && $from_album->get_id() !== $to_album?->id) { + Album::query() + ->where('id', '=', $from_album->get_id()) + ->whereIn('header_id', $photos->map(fn (Photo $p) => $p->id)) + ->update(['header_id' => null]); + } + + $notify = new Notify(); + /** @var Photo $photo */ + foreach ($photos as $photo) { + $notify->do($photo); + } + } +} diff --git a/app/Actions/Photo/Pipes/Duplicate/ReplicateAsPhoto.php b/app/Actions/Photo/Pipes/Duplicate/ReplicateAsPhoto.php deleted file mode 100644 index 62b90bcc3dc..00000000000 --- a/app/Actions/Photo/Pipes/Duplicate/ReplicateAsPhoto.php +++ /dev/null @@ -1,22 +0,0 @@ -replicatePhoto(); - - return $next($state); - } -} diff --git a/app/Actions/Photo/Pipes/Init/FindLivePartner.php b/app/Actions/Photo/Pipes/Init/FindLivePartner.php index 2155d57434e..6ca88863209 100644 --- a/app/Actions/Photo/Pipes/Init/FindLivePartner.php +++ b/app/Actions/Photo/Pipes/Init/FindLivePartner.php @@ -8,6 +8,7 @@ namespace App\Actions\Photo\Pipes\Init; +use App\Constants\PhotoAlbum as PA; use App\Contracts\PhotoCreate\InitPipe; use App\DTO\PhotoCreate\InitDTO; use App\Exceptions\Internal\IllegalOrderOfOperationException; @@ -29,8 +30,10 @@ public function handle(InitDTO $state, \Closure $next): InitDTO // find a potential partner which has the same content id if ($state->exif_info->live_photo_content_id !== null) { $state->live_partner = Photo::query() + ->leftJoin(PA::PHOTO_ALBUM, PA::PHOTO_ID, '=', 'id') ->where('live_photo_content_id', '=', $state->exif_info->live_photo_content_id) - ->where('album_id', '=', $state->album?->get_id()) + ->when($state->album?->get_id() !== null, fn ($q) => $q->where(PA::ALBUM_ID, '=', $state->album->get_id())) + ->when($state->album?->get_id() === null, fn ($q) => $q->whereNull(PA::ALBUM_ID)) ->whereNull('live_photo_short_path')->first(); } diff --git a/app/Actions/Photo/Pipes/Shared/NotifyAlbums.php b/app/Actions/Photo/Pipes/Shared/NotifyAlbums.php index 5e102b9848e..14d49ccd99c 100644 --- a/app/Actions/Photo/Pipes/Shared/NotifyAlbums.php +++ b/app/Actions/Photo/Pipes/Shared/NotifyAlbums.php @@ -9,8 +9,10 @@ namespace App\Actions\Photo\Pipes\Shared; use App\Actions\User\Notify; +use App\Constants\PhotoAlbum as PA; use App\Contracts\PhotoCreate\PhotoDTO; use App\Contracts\PhotoCreate\PhotoPipe; +use Illuminate\Support\Facades\DB; /** * Notify by email if a picture has been added. @@ -19,7 +21,11 @@ class NotifyAlbums implements PhotoPipe { public function handle(PhotoDTO $state, \Closure $next): PhotoDTO { - if ($state->getPhoto()->album_id !== null) { + $count_albums = DB::table(PA::PHOTO_ALBUM) + ->where('photo_id', $state->getPhoto()->id) + ->count(); + + if ($count_albums > 0) { $notify = new Notify(); $notify->do($state->getPhoto()); } diff --git a/app/Actions/Photo/Pipes/Shared/SetOwnership.php b/app/Actions/Photo/Pipes/Shared/SetOwnership.php new file mode 100644 index 00000000000..55bfd898f5d --- /dev/null +++ b/app/Actions/Photo/Pipes/Shared/SetOwnership.php @@ -0,0 +1,24 @@ +photo->owner_id = $state->album instanceof Album ? $state->album->owner_id : $state->intended_owner_id; + + return $next($state); + } +} \ No newline at end of file diff --git a/app/Actions/Photo/Pipes/Shared/SetParent.php b/app/Actions/Photo/Pipes/Shared/SetParent.php new file mode 100644 index 00000000000..aaa6717386a --- /dev/null +++ b/app/Actions/Photo/Pipes/Shared/SetParent.php @@ -0,0 +1,51 @@ +album instanceof Album) { + if ($state->photo->id === null) { + throw new LycheeLogicException('Photo Id is null, cannot set a parent album.'); + } + + // Avoid duplicates key constraint + DB::table(PA::PHOTO_ALBUM) + ->where(PA::PHOTO_ID, '=', $state->photo->id) + ->where(PA::ALBUM_ID, '=', $state->album->id) + ->delete(); + + // Insert the new link + DB::table(PA::PHOTO_ALBUM) + ->insert([ + 'photo_id' => $state->photo->id, + 'album_id' => $state->album->id, + ]); + + // Avoid unnecessary DB request, when we access the album of a + // photo later (e.g. when a notification is sent). + $state->photo->load('albums'); + } + + return $next($state); + } +} \ No newline at end of file diff --git a/app/Actions/Photo/Pipes/Shared/SetParentAndOwnership.php b/app/Actions/Photo/Pipes/Shared/SetParentAndOwnership.php deleted file mode 100644 index 1d709693f14..00000000000 --- a/app/Actions/Photo/Pipes/Shared/SetParentAndOwnership.php +++ /dev/null @@ -1,36 +0,0 @@ -album instanceof Album) { - $state->photo->album_id = $state->album->id; - // Avoid unnecessary DB request, when we access the album of a - // photo later (e.g. when a notification is sent). - $state->photo->setRelation('album', $state->album); - $state->photo->owner_id = $state->album->owner_id; - } else { - $state->photo->album_id = null; - // Avoid unnecessary DB request, when we access the album of a - // photo later (e.g. when a notification is sent). - $state->photo->setRelation('album', null); - $state->photo->owner_id = $state->intended_owner_id; - } - - return $next($state); - } -} \ No newline at end of file diff --git a/app/Actions/RSS/Generate.php b/app/Actions/RSS/Generate.php index cec53f36dfc..599c69836df 100644 --- a/app/Actions/RSS/Generate.php +++ b/app/Actions/RSS/Generate.php @@ -8,52 +8,57 @@ namespace App\Actions\RSS; +use App\Constants\PhotoAlbum as PA; use App\Contracts\Exceptions\InternalLycheeException; +use App\Enum\SizeVariantType; use App\Exceptions\Internal\FrameworkException; use App\Models\Configs; +use App\Models\Extensions\HasUrlGenerator; +use App\Models\Extensions\UTCBasedTimes; use App\Models\Photo; use App\Policies\PhotoQueryPolicy; use Carbon\Exceptions\InvalidFormatException; use Carbon\Exceptions\UnitException; +use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Support\Carbon; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\DB; use Spatie\Feed\FeedItem; +/** + * @template T of object{id:string,title:string,description:?string,type:string,created_at:string,updated_at:string,album_id:string,album_title:string,short_path:string,filesize:int,storage_disk:string,size_variant_type:string,username:string} + */ class Generate { + use HasUrlGenerator; + use UTCBasedTimes; + public function __construct( protected PhotoQueryPolicy $photo_query_policy) { } - private function create_link_to_page(Photo $photo_model): string - { - if ($photo_model->album_id !== null) { - return url('/gallery/' . $photo_model->album_id . '/' . $photo_model->id); - } - - return url('/view?p=' . $photo_model->id); - } - - private function toFeedItem(Photo $photo_model): FeedItem + /** + * @param T $data + * + * @return FeedItem + * + * @throws BindingResolutionException + */ + private function toFeedItem(object $data): FeedItem { - $page_link = $this->create_link_to_page($photo_model); - $size_variant = $photo_model->size_variants->getOriginal(); - $cat_arr = []; - if ($photo_model->album_id !== null) { - $cat_arr[] = $photo_model->album->title; - } + $page_link = route('gallery', ['albumId' => $data->album_id, 'photoId' => $data->id]); $feed_item = [ 'id' => $page_link, - 'title' => $photo_model->title, - 'summary' => $photo_model->description ?? '', - 'updated' => $photo_model->updated_at, + 'title' => $data->title, + 'summary' => $data->description ?? '', + 'updated' => $this->asDateTime($data->updated_at), 'link' => $page_link, - 'enclosure' => $size_variant->url, - 'enclosureType' => $photo_model->type, - 'enclosureLength' => $size_variant->filesize, - 'authorName' => $photo_model->owner->username, - 'category' => $cat_arr, + 'enclosure' => self::pathToUrl($data->short_path, $data->storage_disk, SizeVariantType::ORIGINAL), + 'enclosureType' => $data->type, + 'enclosureLength' => $data->filesize, + 'authorName' => $data->username, + 'category' => [$data->album_title], ]; return FeedItem::create($feed_item); @@ -74,17 +79,36 @@ public function do(): Collection throw new FrameworkException('Date/Time component (Carbon)', $e); } - /** @var Collection $photos */ + /** @var Collection $photos */ $photos = $this->photo_query_policy ->applySearchabilityFilter( - query: Photo::query()->with(['album', 'owner', 'size_variants']), + query: Photo::query(), origin: null, include_nsfw: !Configs::getValueAsBool('hide_nsfw_in_rss') ) + ->joinSub(DB::table(PA::PHOTO_ALBUM), 'outer_' . PA::PHOTO_ALBUM, 'photos.id', '=', 'outer_' . PA::PHOTO_ID) + ->join('base_albums', 'base_albums.id', '=', 'outer_' . PA::ALBUM_ID) + ->join('size_variants', 'size_variants.photo_id', '=', 'photos.id') + ->join('users', 'users.id', '=', 'photos.owner_id') + ->where('size_variants.type', '=', SizeVariantType::ORIGINAL->value) + ->select([ + 'photos.id', + 'photos.title', + 'photos.description', + 'photos.type', + 'photos.updated_at', + 'outer_' . PA::ALBUM_ID, + 'base_albums.title as album_title', + 'size_variants.short_path', + 'size_variants.filesize', + 'size_variants.storage_disk', + 'users.username'] + ) ->where('photos.created_at', '>=', $now_minus) ->limit($rss_max) + ->toBase() // We use toBase() to avoid the use of the Eloquent casts etc. ->get(); - return $photos->map(fn (Photo $p) => $this->toFeedItem($p)); + return $photos->map(fn (object $p) => $this->toFeedItem($p)); } } \ No newline at end of file diff --git a/app/Actions/Search/PhotoSearch.php b/app/Actions/Search/PhotoSearch.php index 706531df9ef..b0eb9aab3da 100644 --- a/app/Actions/Search/PhotoSearch.php +++ b/app/Actions/Search/PhotoSearch.php @@ -57,7 +57,7 @@ public function query(array $terms): Collection public function sqlQuery(array $terms, ?Album $album = null): Builder { $query = $this->photoQueryPolicy->applySearchabilityFilter( - query: Photo::query()->with(['album', 'statistics', 'size_variants', 'palette']), + query: Photo::query()->with(['albums', 'statistics', 'size_variants', 'palette']), origin: $album, include_nsfw: !Configs::getValueAsBool('hide_nsfw_in_search') ); diff --git a/app/Actions/Statistics/Spaces.php b/app/Actions/Statistics/Spaces.php index caca4552206..9f387a7457d 100644 --- a/app/Actions/Statistics/Spaces.php +++ b/app/Actions/Statistics/Spaces.php @@ -8,6 +8,7 @@ namespace App\Actions\Statistics; +use App\Constants\PhotoAlbum as PA; use App\Enum\SizeVariantType; use Illuminate\Database\Query\JoinClause; use Illuminate\Support\Collection; @@ -116,13 +117,7 @@ public function getSpacePerSizeVariantTypePerAlbum(string $album_id): Collection ->on('albums._rgt', '>=', 'descendants._rgt'); } ) - ->joinSub( - query: DB::table('photos'), - as: 'photos', - first: 'photos.album_id', - operator: '=', - second: 'descendants.id', - ) + ->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'descendants.id') ->joinSub( query: DB::table('size_variants') ->select(['size_variants.id', 'size_variants.photo_id', 'size_variants.type', 'size_variants.filesize']) @@ -130,7 +125,7 @@ public function getSpacePerSizeVariantTypePerAlbum(string $album_id): Collection as: 'size_variants', first: 'size_variants.photo_id', operator: '=', - second: 'photos.id', + second: PA::PHOTO_ID, ) ->select( 'size_variants.type', @@ -176,14 +171,8 @@ public function getSpacePerAlbum(?string $album_id = null, ?int $owner_id = null operator: '=', second: 'albums.id' ) - ->where('base_albums.owner_id', '=', $owner_id)) - ->joinSub( - query: DB::table('photos'), - as: 'photos', - first: 'photos.album_id', - operator: '=', - second: 'albums.id' - ) + ->where('base_albums.owner_id', '=', $owner_id)) + ->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'albums.id') ->joinSub( query: DB::table('size_variants') ->select(['size_variants.id', 'size_variants.photo_id', 'size_variants.filesize']) @@ -191,7 +180,7 @@ public function getSpacePerAlbum(?string $album_id = null, ?int $owner_id = null as: 'size_variants', first: 'size_variants.photo_id', operator: '=', - second: 'photos.id' + second: PA::PHOTO_ID ) ->select( 'albums.id', @@ -239,13 +228,7 @@ public function getTotalSpacePerAlbum(?string $album_id = null, ?int $owner_id = ->on('albums._rgt', '>=', 'descendants._rgt'); } ) - ->joinSub( - query: DB::table('photos'), - as: 'photos', - first: 'photos.album_id', - operator: '=', - second: 'descendants.id' - ) + ->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'descendants.id') ->joinSub( query: DB::table('size_variants') ->select(['size_variants.id', 'size_variants.photo_id', 'size_variants.filesize']) @@ -253,7 +236,7 @@ public function getTotalSpacePerAlbum(?string $album_id = null, ?int $owner_id = as: 'size_variants', first: 'size_variants.photo_id', operator: '=', - second: 'photos.id' + second: PA::PHOTO_ID ) ->select( 'albums.id', @@ -304,13 +287,7 @@ public function getPhotoCountPerAlbum(?string $album_id = null, ?int $owner_id = second: 'albums.id' ) ->when($owner_id !== null, fn ($query) => $query->where('base_albums.owner_id', '=', $owner_id)) - ->joinSub( - query: DB::table('photos')->select(['photos.id', 'photos.album_id']), - as: 'photos', - first: 'photos.album_id', - operator: '=', - second: 'albums.id' - ) + ->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'albums.id') ->joinSub( query: DB::table('users')->select(['users.id', 'users.username']), as: 'users', @@ -325,7 +302,7 @@ public function getPhotoCountPerAlbum(?string $album_id = null, ?int $owner_id = 'base_albums.is_nsfw', 'albums._lft', 'albums._rgt', - DB::raw('COUNT(photos.id) as num_photos'), + DB::raw('COUNT(' . PA::PHOTO_ID . ') as num_photos'), )->groupBy( 'albums.id', 'username', @@ -378,13 +355,7 @@ public function getTotalPhotoCountPerAlbum(?string $album_id = null, ?int $owner ->on('albums._rgt', '>=', 'descendants._rgt'); } ) - ->joinSub( - query: DB::table('photos')->select(['photos.id', 'photos.album_id']), - as: 'photos', - first: 'photos.album_id', - operator: '=', - second: 'descendants.id' - ) + ->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'descendants.id') ->joinSub( query: DB::table('users')->select(['users.id', 'users.username']), as: 'users', @@ -399,7 +370,7 @@ public function getTotalPhotoCountPerAlbum(?string $album_id = null, ?int $owner 'base_albums.is_nsfw', 'albums._lft', 'albums._rgt', - DB::raw('COUNT(photos.id) as num_photos'), + DB::raw('COUNT(' . PA::PHOTO_ID . ') as num_photos'), )->groupBy( 'albums.id', 'username', diff --git a/app/Actions/User/Notify.php b/app/Actions/User/Notify.php index fecb2733351..86655c535e8 100644 --- a/app/Actions/User/Notify.php +++ b/app/Actions/User/Notify.php @@ -8,8 +8,11 @@ namespace App\Actions\User; +use App\Constants\AccessPermissionConstants as APC; +use App\Constants\PhotoAlbum as PA; use App\Exceptions\ConfigurationKeyMissingException; use App\Exceptions\Internal\QueryBuilderException; +use App\Models\Album; use App\Models\Configs; use App\Models\Photo; use App\Models\User; @@ -42,10 +45,16 @@ public function do(Photo $photo): void // Admin user is always notified $users = User::query()->where('may_administrate', '=', true)->get(); - $album = $photo->album; - if ($album !== null) { - $users = $users->concat($album->shared_with); - $users->push($album->owner); + $albums = Album::query()->without(['thumbs', 'statistics', 'cover'])->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'album.id') + ->where(PA::PHOTO_ID, '=', $photo->id) + ->get(); + + if ($albums->count() > 0) { + $shared_with = User::query()->join(APC::ACCESS_PERMISSIONS, APC::USER_ID, '=', 'user.id') + ->whereIn(APC::BASE_ALBUM_ID, $albums->pluck('id')) + ->get(); + $users->push(...$shared_with->all()); + $users->push(...$albums->map(fn ($a) => $a->owner)->all()); } $users = $users diff --git a/app/Console/Commands/PhotosAddedNotification.php b/app/Console/Commands/PhotosAddedNotification.php index ce2328c8be8..56b09aa7ded 100644 --- a/app/Console/Commands/PhotosAddedNotification.php +++ b/app/Console/Commands/PhotosAddedNotification.php @@ -8,7 +8,9 @@ namespace App\Console\Commands; +use App\Constants\PhotoAlbum as PA; use App\Mail\PhotosAdded; +use App\Models\BaseAlbumImpl; use App\Models\Configs; use App\Models\Photo; use App\Models\User; @@ -54,13 +56,6 @@ public function handle(): int ->find($notification->data['id']); if ($photo !== null) { - if (!isset($photos[$photo->album_id])) { - $photos[$photo->album_id] = [ - 'name' => $photo->album->title, - 'photos' => [], - ]; - } - $thumb_url = $photo->size_variants->getThumb()?->url; // Mail clients do not like relative paths. @@ -69,19 +64,26 @@ public function handle(): int $thumb_url = URL::asset($thumb_url); } - // If the url config doesn't contain a trailing slash then add it - if (str_ends_with(config('app.url'), '/')) { - $trailing_slash = ''; - } else { - $trailing_slash = '/'; - } + BaseAlbumImpl::query()->join(PA::PHOTO_ALBUM, PA::ALBUM_ID, '=', 'base_albums.id') + ->where(PA::PHOTO_ID, '=', $photo->id) + ->get() + ->each(function (BaseAlbumImpl $album) use (&$photos, $photo, $thumb_url): void { + $album_id = $album->id; + $title = $album->title; + + if (!isset($photos[$album_id])) { + $photos[$album_id] = [ + 'name' => $title, + 'photos' => [], + ]; + } - $photos[$photo->album_id]['photos'][$photo->id] = [ - 'title' => $photo->title, - 'thumb' => $thumb_url, - // TODO: Clean this up. There should be a better way to get the URL of a photo than constructing it manually - 'link' => config('app.url') . $trailing_slash . 'r/' . $photo->album_id . '/' . $photo->id, - ]; + $photos[$album_id]['photos'][$photo->id] = [ + 'title' => $photo->title, + 'thumb' => $thumb_url, + 'link' => route('gallery', ['albumId' => $album_id, 'photoId' => $photo->id]), + ]; + }); } } @@ -93,4 +95,4 @@ public function handle(): int return 0; } -} \ No newline at end of file +} diff --git a/app/Constants/PhotoAlbum.php b/app/Constants/PhotoAlbum.php new file mode 100644 index 00000000000..a1b3ce1868d --- /dev/null +++ b/app/Constants/PhotoAlbum.php @@ -0,0 +1,40 @@ +table !== self::PHOTO_ALBUM) { + return false; + } + // We now need to check if the join column is correct. + + /** @var array{type:string,first:string,operator:string,second:string}[] */ + $wheres = $join->wheres ?? []; + foreach ($wheres as $where) { + if (str_contains($where['first'], 'photo_id') || str_contains($where['second'], 'photo_id')) { + return true; + } + } + + // Likely to not be the join we want (e.g. on the table). + return false; + } +} \ No newline at end of file diff --git a/app/Contracts/Http/Requests/HasFromAlbum.php b/app/Contracts/Http/Requests/HasFromAlbum.php new file mode 100644 index 00000000000..dd232b8c66a --- /dev/null +++ b/app/Contracts/Http/Requests/HasFromAlbum.php @@ -0,0 +1,19 @@ +has_been_resynced = $val; } - - public function replicatePhoto(): void - { - $dup = $this->photo; - $this->photo = $dup->replicate(); - } } diff --git a/app/Http/Controllers/Gallery/FrameController.php b/app/Http/Controllers/Gallery/FrameController.php index 8f924d6bc75..48d5ac51190 100644 --- a/app/Http/Controllers/Gallery/FrameController.php +++ b/app/Http/Controllers/Gallery/FrameController.php @@ -63,7 +63,7 @@ public function random(FrameRequest $request): PhotoResource { $photo = $this->loadPhoto($request->album(), 5); - return PhotoResource::fromModel($photo); + return new PhotoResource($photo, $request->album()); } /** @@ -86,12 +86,12 @@ private function loadPhoto(AbstractAlbum|null $album, int $retries = 5): ?Photo // default query if ($album === null) { $query = $this->photo_query_policy->applySearchabilityFilter( - query: Photo::query()->with(['album', 'size_variants', 'palette']), + query: Photo::query()->with(['albums', 'size_variants', 'palette']), origin: null, include_nsfw: !Configs::getValueAsBool('hide_nsfw_in_frame') ); } else { - $query = $album->photos()->with(['album', 'size_variants', 'palette']); + $query = $album->photos()->with(['albums', 'size_variants', 'palette']); } /** @var ?Photo $photo */ diff --git a/app/Http/Controllers/Gallery/PhotoController.php b/app/Http/Controllers/Gallery/PhotoController.php index d1f94a7ad4e..3af23139c37 100644 --- a/app/Http/Controllers/Gallery/PhotoController.php +++ b/app/Http/Controllers/Gallery/PhotoController.php @@ -10,8 +10,7 @@ use App\Actions\Import\FromUrl; use App\Actions\Photo\Delete; -use App\Actions\Photo\Duplicate; -use App\Actions\Photo\Move; +use App\Actions\Photo\MoveOrDuplicate; use App\Actions\Photo\Rotate; use App\Constants\FileSystem; use App\Contracts\Models\AbstractAlbum; @@ -131,7 +130,7 @@ public function update(EditPhotoRequest $request): PhotoResource $photo->save(); - return PhotoResource::fromModel($photo); + return new PhotoResource($photo, $request->from_album()); } /** @@ -148,9 +147,13 @@ public function star(SetPhotosStarredRequest $request): void /** * Moves the photos to an album. */ - public function move(MovePhotosRequest $request, Move $move): void + public function move(MovePhotosRequest $request, MoveOrDuplicate $move): void { - $move->do($request->photos(), $request->album()); + $move->do( + photos: $request->photos(), + from_album: $request->from_album(), + to_album: $request->album() + ); } /** @@ -158,7 +161,7 @@ public function move(MovePhotosRequest $request, Move $move): void */ public function delete(DeletePhotosRequest $request, Delete $delete): void { - $file_deleter = $delete->do($request->photoIds()); + $file_deleter = $delete->do($request->photoIds(), $request->from_id()); App::terminating(fn () => $file_deleter->do()); } @@ -174,16 +177,16 @@ public function rotate(RotatePhotoRequest $request): PhotoResource $rotate_strategy = new Rotate($request->photo(), $request->direction()); $photo = $rotate_strategy->do(); - return PhotoResource::fromModel($photo); + return new PhotoResource($photo, $request->from_album()); } /** * Copy a photos to an album. * Only the SQL entry is duplicated for space reason. */ - public function copy(CopyPhotosRequest $request, Duplicate $duplicate): void + public function copy(CopyPhotosRequest $request, MoveOrDuplicate $duplicate): void { - $duplicate->do($request->photos(), $request->album()); + $duplicate->do($request->photos(), $request->album(), $request->album()); } /** diff --git a/app/Http/Controllers/VueController.php b/app/Http/Controllers/VueController.php index b1bad8e1661..8396e5fe449 100644 --- a/app/Http/Controllers/VueController.php +++ b/app/Http/Controllers/VueController.php @@ -58,7 +58,7 @@ public function gallery(?string $album_id = null, ?string $photo_id = null): Vie } if ($photo_id !== null) { - $photo = Photo::findOrFail($photo_id); + $photo = Photo::with('albums')->findOrFail($photo_id); Gate::authorize(PhotoPolicy::CAN_SEE, [Photo::class, $photo]); session()->now('photo', $photo); } diff --git a/app/Http/Requests/Album/DeleteAlbumsRequest.php b/app/Http/Requests/Album/DeleteAlbumsRequest.php index 4e382210cf9..3e9e8d6cae6 100644 --- a/app/Http/Requests/Album/DeleteAlbumsRequest.php +++ b/app/Http/Requests/Album/DeleteAlbumsRequest.php @@ -16,7 +16,7 @@ use App\Http\Requests\Traits\HasAlbumIdsTrait; use App\Http\Requests\Traits\HasBaseAlbumTrait; use App\Policies\AlbumPolicy; -use App\Rules\AlbumIDRule; +use App\Rules\RandomIDRule; use Illuminate\Support\Facades\Gate; class DeleteAlbumsRequest extends BaseApiRequest implements HasBaseAlbum, HasAlbumIds @@ -39,7 +39,7 @@ public function rules(): array { return [ RequestAttribute::ALBUM_IDS_ATTRIBUTE => 'required|array|min:1', - RequestAttribute::ALBUM_IDS_ATTRIBUTE . '.*' => ['required', new AlbumIDRule(false)], + RequestAttribute::ALBUM_IDS_ATTRIBUTE . '.*' => ['required', new RandomIDRule(false)], ]; } diff --git a/app/Http/Requests/Album/SetAsHeaderRequest.php b/app/Http/Requests/Album/SetAsHeaderRequest.php index 89786bba98a..b464fe46875 100644 --- a/app/Http/Requests/Album/SetAsHeaderRequest.php +++ b/app/Http/Requests/Album/SetAsHeaderRequest.php @@ -8,6 +8,7 @@ namespace App\Http\Requests\Album; +use App\Constants\PhotoAlbum as PA; use App\Contracts\Http\Requests\HasAlbum; use App\Contracts\Http\Requests\HasCompactBoolean; use App\Contracts\Http\Requests\HasPhoto; @@ -21,6 +22,7 @@ use App\Models\Photo; use App\Policies\AlbumPolicy; use App\Rules\RandomIDRule; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Gate; use Illuminate\Validation\ValidationException; @@ -30,10 +32,17 @@ class SetAsHeaderRequest extends BaseApiRequest implements HasAlbum, HasPhoto, H use HasPhotoTrait; use HasCompactBooleanTrait; + private array $album_ids = []; + public function authorize(): bool { + $is_photo_in_album = $this->photo !== null && DB::table(PA::PHOTO_ALBUM) + ->where(PA::ALBUM_ID, '=', $this->album->id) + ->where(PA::PHOTO_ID, '=', $this->photo->id) + ->count() > 0; + return Gate::check(AlbumPolicy::CAN_EDIT, [AbstractAlbum::class, $this->album]) && - ($this->is_compact || ($this->photo->album_id === $this->album->id)); + ($this->is_compact || $is_photo_in_album); } /** diff --git a/app/Http/Requests/Album/UpdateAlbumRequest.php b/app/Http/Requests/Album/UpdateAlbumRequest.php index f1697907808..e80c161dc6a 100644 --- a/app/Http/Requests/Album/UpdateAlbumRequest.php +++ b/app/Http/Requests/Album/UpdateAlbumRequest.php @@ -8,6 +8,7 @@ namespace App\Http\Requests\Album; +use App\Constants\PhotoAlbum as PA; use App\Contracts\Http\Requests\HasAlbum; use App\Contracts\Http\Requests\HasAlbumSortingCriterion; use App\Contracts\Http\Requests\HasCompactBoolean; @@ -54,6 +55,7 @@ use App\Rules\EnumRequireSupportRule; use App\Rules\RandomIDRule; use App\Rules\TitleRule; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Gate; use Illuminate\Validation\Rules\Enum; use Illuminate\Validation\ValidationException; @@ -77,9 +79,13 @@ class UpdateAlbumRequest extends BaseApiRequest implements HasAlbum, HasTitle, H public function authorize(): bool { return Gate::check(AlbumPolicy::CAN_EDIT, [AbstractAlbum::class, $this->album]) && - ($this->is_compact || - $this->photo === null || - $this->photo->album_id === $this->album->id); + ( + $this->is_compact || + $this->photo === null || + (DB::table(PA::PHOTO_ALBUM)->where('album_id', $this->album->id) + ->where('photo_id', $this->photo->id) + ->count() > 1) + ); } /** @@ -95,12 +101,14 @@ public function rules(): array RequestAttribute::PHOTO_SORTING_COLUMN_ATTRIBUTE => ['present', 'nullable', new Enum(ColumnSortingPhotoType::class)], RequestAttribute::PHOTO_SORTING_ORDER_ATTRIBUTE => [ 'required_with:' . RequestAttribute::PHOTO_SORTING_COLUMN_ATTRIBUTE, - 'nullable', new Enum(OrderSortingType::class), + 'nullable', + new Enum(OrderSortingType::class), ], RequestAttribute::ALBUM_SORTING_COLUMN_ATTRIBUTE => ['present', 'nullable', new Enum(ColumnSortingAlbumType::class)], RequestAttribute::ALBUM_SORTING_ORDER_ATTRIBUTE => [ 'required_with:' . RequestAttribute::ALBUM_SORTING_COLUMN_ATTRIBUTE, - 'nullable', new Enum(OrderSortingType::class), + 'nullable', + new Enum(OrderSortingType::class), ], RequestAttribute::ALBUM_ASPECT_RATIO_ATTRIBUTE => ['present', 'nullable', new Enum(AspectRatioType::class)], RequestAttribute::ALBUM_PHOTO_LAYOUT => ['present', 'nullable', new Enum(PhotoLayoutType::class)], @@ -161,4 +169,4 @@ protected function processValidatedValues(array $values, array $files): void $photo_id = $values[RequestAttribute::HEADER_ID_ATTRIBUTE]; $this->photo = $photo_id !== null ? Photo::query()->findOrFail($photo_id) : null; } -} \ No newline at end of file +} diff --git a/app/Http/Requests/Album/ZipRequest.php b/app/Http/Requests/Album/ZipRequest.php index 540631226f5..447dd350ed0 100644 --- a/app/Http/Requests/Album/ZipRequest.php +++ b/app/Http/Requests/Album/ZipRequest.php @@ -128,7 +128,7 @@ private function processPhotos(array $photo_ids): void return; } - $photo_query = Photo::query()->with(['album']); + $photo_query = Photo::query()->with(['albums']); // The condition is required, because Lychee also supports to archive // the "live video" as a size variant which is not a proper size variant $variant = $this->size_variant->getSizeVariantType(); diff --git a/app/Http/Requests/Photo/DeletePhotosRequest.php b/app/Http/Requests/Photo/DeletePhotosRequest.php index 30d8a2e8f53..f0260856b01 100644 --- a/app/Http/Requests/Photo/DeletePhotosRequest.php +++ b/app/Http/Requests/Photo/DeletePhotosRequest.php @@ -8,18 +8,22 @@ namespace App\Http\Requests\Photo; +use App\Contracts\Http\Requests\HasFromId; use App\Contracts\Http\Requests\HasPhotoIds; use App\Contracts\Http\Requests\RequestAttribute; use App\Http\Requests\BaseApiRequest; +use App\Http\Requests\Traits\HasFromIdTrait; use App\Http\Requests\Traits\HasPhotoIdsTrait; use App\Models\Photo; use App\Policies\PhotoPolicy; +use App\Rules\AlbumIDRule; use App\Rules\RandomIDRule; use Illuminate\Support\Facades\Gate; -class DeletePhotosRequest extends BaseApiRequest implements HasPhotoIds +class DeletePhotosRequest extends BaseApiRequest implements HasPhotoIds, HasFromId { use HasPhotoIdsTrait; + use HasFromIdTrait; /** * {@inheritDoc} @@ -37,6 +41,7 @@ public function rules(): array return [ RequestAttribute::PHOTO_IDS_ATTRIBUTE => 'required|array|min:1', RequestAttribute::PHOTO_IDS_ATTRIBUTE . '.*' => ['required', new RandomIDRule(false)], + RequestAttribute::FROM_ID_ATTRIBUTE => ['required', new AlbumIDRule(false)], ]; } @@ -49,5 +54,6 @@ protected function processValidatedValues(array $values, array $files): void // models for efficiency reasons. // Instead, we use mass deletion via low-level SQL queries later. $this->photo_ids = $values[RequestAttribute::PHOTO_IDS_ATTRIBUTE]; + $this->from_id = $values[RequestAttribute::FROM_ID_ATTRIBUTE]; } } diff --git a/app/Http/Requests/Photo/EditPhotoRequest.php b/app/Http/Requests/Photo/EditPhotoRequest.php index bb9551d9084..f058f58eb72 100644 --- a/app/Http/Requests/Photo/EditPhotoRequest.php +++ b/app/Http/Requests/Photo/EditPhotoRequest.php @@ -9,6 +9,7 @@ namespace App\Http\Requests\Photo; use App\Contracts\Http\Requests\HasDescription; +use App\Contracts\Http\Requests\HasFromAlbum; use App\Contracts\Http\Requests\HasLicense; use App\Contracts\Http\Requests\HasPhoto; use App\Contracts\Http\Requests\HasTags; @@ -19,6 +20,7 @@ use App\Enum\LicenseType; use App\Http\Requests\BaseApiRequest; use App\Http\Requests\Traits\HasDescriptionTrait; +use App\Http\Requests\Traits\HasFromAlbumTrait; use App\Http\Requests\Traits\HasLicenseTrait; use App\Http\Requests\Traits\HasPhotoTrait; use App\Http\Requests\Traits\HasTagsTrait; @@ -34,7 +36,7 @@ use Illuminate\Support\Facades\Gate; use Illuminate\Validation\Rules\Enum; -class EditPhotoRequest extends BaseApiRequest implements HasPhoto, HasTags, HasUploadDate, HasDescription, HasLicense, HasTitle, HasTakenAt +class EditPhotoRequest extends BaseApiRequest implements HasPhoto, HasTags, HasUploadDate, HasDescription, HasLicense, HasTitle, HasTakenAt, HasFromAlbum { use HasPhotoTrait; use HasTitleTrait; @@ -43,6 +45,7 @@ class EditPhotoRequest extends BaseApiRequest implements HasPhoto, HasTags, HasU use HasUploadDateTrait; use HasLicenseTrait; use HasTakenAtDateTrait; + use HasFromAlbumTrait; /** * {@inheritDoc} @@ -66,6 +69,7 @@ public function rules(): array RequestAttribute::LICENSE_ATTRIBUTE => ['required', new Enum(LicenseType::class)], RequestAttribute::UPLOAD_DATE_ATTRIBUTE => ['required', 'date'], RequestAttribute::TAKEN_DATE_ATTRIBUTE => ['nullable', 'date'], + RequestAttribute::FROM_ID_ATTRIBUTE => ['present', new RandomIDRule(true)], ]; } @@ -78,7 +82,7 @@ protected function processValidatedValues(array $values, array $files): void $photo_id = $values[RequestAttribute::PHOTO_ID_ATTRIBUTE]; $this->photo = Photo::query() - ->with(['size_variants']) + ->with(['size_variants', 'albums']) ->findOrFail($photo_id); $this->title = $values[RequestAttribute::TITLE_ATTRIBUTE]; @@ -91,5 +95,7 @@ protected function processValidatedValues(array $values, array $files): void if (isset($values[RequestAttribute::TAKEN_DATE_ATTRIBUTE])) { $this->taken_at = Carbon::parse($values[RequestAttribute::TAKEN_DATE_ATTRIBUTE]); } + + $this->from_album = $this->album_factory->findNullalbleAbstractAlbumOrFail($values[RequestAttribute::FROM_ID_ATTRIBUTE]); } } \ No newline at end of file diff --git a/app/Http/Requests/Photo/MovePhotosRequest.php b/app/Http/Requests/Photo/MovePhotosRequest.php index afdab1aeaf6..0c08ccb35ee 100644 --- a/app/Http/Requests/Photo/MovePhotosRequest.php +++ b/app/Http/Requests/Photo/MovePhotosRequest.php @@ -9,22 +9,54 @@ namespace App\Http\Requests\Photo; use App\Contracts\Http\Requests\HasAlbum; +use App\Contracts\Http\Requests\HasFromAlbum; use App\Contracts\Http\Requests\HasPhotos; use App\Contracts\Http\Requests\RequestAttribute; +use App\Contracts\Models\AbstractAlbum; use App\Http\Requests\BaseApiRequest; use App\Http\Requests\Traits\Authorize\AuthorizeCanEditPhotosAlbumTrait; use App\Http\Requests\Traits\HasAlbumTrait; +use App\Http\Requests\Traits\HasFromAlbumTrait; use App\Http\Requests\Traits\HasPhotosTrait; use App\Models\Album; use App\Models\Photo; +use App\Policies\AlbumPolicy; +use App\Policies\PhotoPolicy; +use App\Rules\AlbumIDRule; use App\Rules\RandomIDRule; +use Illuminate\Support\Facades\Gate; -class MovePhotosRequest extends BaseApiRequest implements HasPhotos, HasAlbum +class MovePhotosRequest extends BaseApiRequest implements HasPhotos, HasAlbum, HasFromAlbum { use HasPhotosTrait; use HasAlbumTrait; + use HasFromAlbumTrait; use AuthorizeCanEditPhotosAlbumTrait; + /** + * {@inheritDoc} + */ + public function authorize(): bool + { + if (!Gate::check(AlbumPolicy::CAN_EDIT, [AbstractAlbum::class, $this->album])) { + return false; + } + + if (!Gate::check(AlbumPolicy::CAN_EDIT, [AbstractAlbum::class, $this->from_album])) { + return false; + } + + // TODO: refactor this check so it does not explode. + /** @var Photo $photo */ + foreach ($this->photos as $photo) { + if (!Gate::check(PhotoPolicy::CAN_EDIT, $photo)) { + return false; + } + } + + return true; + } + /** * {@inheritDoc} */ @@ -34,6 +66,7 @@ public function rules(): array RequestAttribute::PHOTO_IDS_ATTRIBUTE => 'required|array|min:1', RequestAttribute::PHOTO_IDS_ATTRIBUTE . '.*' => ['required', new RandomIDRule(false)], RequestAttribute::ALBUM_ID_ATTRIBUTE => ['present', new RandomIDRule(true)], + RequestAttribute::FROM_ID_ATTRIBUTE => ['present', new AlbumIDRule(true)], ]; } @@ -46,8 +79,11 @@ protected function processValidatedValues(array $values, array $files): void $photos_ids = $values[RequestAttribute::PHOTO_IDS_ATTRIBUTE]; $this->photos = Photo::query() ->findOrFail($photos_ids); + /** @var string|null */ $target_album_id = $values[RequestAttribute::ALBUM_ID_ATTRIBUTE]; $this->album = $target_album_id === null ? null : Album::query()->findOrFail($target_album_id); + + $this->from_album = $this->album_factory->findNullalbleAbstractAlbumOrFail($values[RequestAttribute::FROM_ID_ATTRIBUTE]); } } \ No newline at end of file diff --git a/app/Http/Requests/Photo/RotatePhotoRequest.php b/app/Http/Requests/Photo/RotatePhotoRequest.php index d7cad6892f4..907b73ca81d 100644 --- a/app/Http/Requests/Photo/RotatePhotoRequest.php +++ b/app/Http/Requests/Photo/RotatePhotoRequest.php @@ -8,19 +8,22 @@ namespace App\Http\Requests\Photo; +use App\Contracts\Http\Requests\HasFromAlbum; use App\Contracts\Http\Requests\HasPhoto; use App\Contracts\Http\Requests\RequestAttribute; use App\Http\Requests\BaseApiRequest; use App\Http\Requests\Traits\Authorize\AuthorizeCanEditPhotoTrait; +use App\Http\Requests\Traits\HasFromAlbumTrait; use App\Http\Requests\Traits\HasPhotoTrait; use App\Models\Photo; use App\Rules\RandomIDRule; use Illuminate\Validation\Rule; -class RotatePhotoRequest extends BaseApiRequest implements HasPhoto +class RotatePhotoRequest extends BaseApiRequest implements HasPhoto, HasFromAlbum { use HasPhotoTrait; use AuthorizeCanEditPhotoTrait; + use HasFromAlbumTrait; protected int $direction; @@ -32,6 +35,7 @@ public function rules(): array return [ RequestAttribute::PHOTO_ID_ATTRIBUTE => ['required', new RandomIDRule(false)], RequestAttribute::DIRECTION_ATTRIBUTE => ['required', Rule::in([-1, 1])], + RequestAttribute::FROM_ID_ATTRIBUTE => ['present', new RandomIDRule(true)], ]; } @@ -43,9 +47,11 @@ protected function processValidatedValues(array $values, array $files): void /** @var ?string $photo_id */ $photo_id = $values[RequestAttribute::PHOTO_ID_ATTRIBUTE]; $this->photo = Photo::query() - ->with(['size_variants']) + ->with(['size_variants', 'albums']) ->findOrFail($photo_id); $this->direction = intval($values[RequestAttribute::DIRECTION_ATTRIBUTE]); + + $this->from_album = $this->album_factory->findNullalbleAbstractAlbumOrFail($values[RequestAttribute::FROM_ID_ATTRIBUTE]); } public function direction(): int diff --git a/app/Http/Requests/Traits/HasFromAlbumTrait.php b/app/Http/Requests/Traits/HasFromAlbumTrait.php new file mode 100644 index 00000000000..b3ca08d1c9b --- /dev/null +++ b/app/Http/Requests/Traits/HasFromAlbumTrait.php @@ -0,0 +1,24 @@ +from_album; + } +} diff --git a/app/Http/Resources/Collections/PositionDataResource.php b/app/Http/Resources/Collections/PositionDataResource.php index 8c4d2d5eeb8..e73bc58040a 100644 --- a/app/Http/Resources/Collections/PositionDataResource.php +++ b/app/Http/Resources/Collections/PositionDataResource.php @@ -8,7 +8,9 @@ namespace App\Http\Resources\Collections; +use App\Contracts\Models\AbstractAlbum; use App\Http\Resources\Models\PhotoResource; +use App\Http\Resources\Traits\HasPrepPhotoCollection; use Illuminate\Support\Collection; use Spatie\LaravelData\Data; use Spatie\TypeScriptTransformer\Attributes\LiteralTypeScriptType; @@ -20,6 +22,8 @@ #[TypeScript()] class PositionDataResource extends Data { + use HasPrepPhotoCollection; + public ?string $id; public ?string $title; public ?string $track_url; @@ -28,20 +32,18 @@ class PositionDataResource extends Data public ?Collection $photos; /** - * @param string|null $id the ID of the album; `null` for root album - * @param string|null $title the title of the album; `null` if untitled + * @param AbstractAlbum|null $album the album; `null` for root album * @param Collection $photos the collection of photos with position data to be shown on map * @param string|null $track_url the URL of the album's track */ public function __construct( - ?string $id, - ?string $title, + ?AbstractAlbum $album, Collection $photos, ?string $track_url, ) { - $this->id = $id; - $this->title = $title; + $this->id = $album?->get_id(); + $this->title = $album?->get_title(); $this->track_url = $track_url; - $this->photos = PhotoResource::collect($photos); + $this->photos = $this->toPhotoResources($photos, $album); } } diff --git a/app/Http/Resources/Models/AlbumResource.php b/app/Http/Resources/Models/AlbumResource.php index dd39ed08304..c8683574656 100644 --- a/app/Http/Resources/Models/AlbumResource.php +++ b/app/Http/Resources/Models/AlbumResource.php @@ -85,7 +85,7 @@ public function __construct(Album $album) $this->parent_id = $album->parent_id; $this->has_albums = !$album->isLeaf(); $this->albums = $album->relationLoaded('children') ? ThumbAlbumResource::collect($album->children) : null; - $this->photos = $album->relationLoaded('photos') ? PhotoResource::collect($album->photos) : null; + $this->photos = $album->relationLoaded('photos') ? $this->toPhotoResources($album->photos, $album) : null; if ($this->photos !== null) { // Prep collection with first and last link + which id is next. $this->prepPhotosCollection(); diff --git a/app/Http/Resources/Models/LiveMetricsResource.php b/app/Http/Resources/Models/LiveMetricsResource.php index f692550f693..8f25c5ecdb9 100644 --- a/app/Http/Resources/Models/LiveMetricsResource.php +++ b/app/Http/Resources/Models/LiveMetricsResource.php @@ -21,8 +21,8 @@ public function __construct( public string $visitor_id, public MetricsAction $action, public ?string $photo_id, - public ?string $album_id, - public ?string $title, + public string $album_id, + public string $title, public ?string $url = null, ) { } diff --git a/app/Http/Resources/Models/PhotoResource.php b/app/Http/Resources/Models/PhotoResource.php index 49b1a43a38c..c7485b8b346 100644 --- a/app/Http/Resources/Models/PhotoResource.php +++ b/app/Http/Resources/Models/PhotoResource.php @@ -8,6 +8,7 @@ namespace App\Http\Resources\Models; +use App\Contracts\Models\AbstractAlbum; use App\Enum\LicenseType; use App\Http\Resources\Models\Utils\PreComputedPhotoData; use App\Http\Resources\Models\Utils\PreformattedPhotoData; @@ -67,10 +68,10 @@ class PhotoResource extends Data public ?PhotoStatisticsResource $statistics = null; - public function __construct(Photo $photo) + public function __construct(Photo $photo, ?AbstractAlbum $album) { $this->id = $photo->id; - $this->album_id = $photo->album_id; + $this->album_id = $album?->get_id() ?? null; $this->altitude = $photo->altitude; $this->aperture = $photo->aperture; $this->checksum = $photo->checksum; @@ -91,14 +92,14 @@ public function __construct(Photo $photo) $this->model = $photo->model; $this->original_checksum = $photo->original_checksum; $this->shutter = $photo->shutter; - $this->size_variants = new SizeVariantsResouce($photo); + $this->size_variants = new SizeVariantsResouce($photo, $album); $this->tags = $photo->tags; $this->taken_at = $photo->taken_at?->toIso8601String(); $this->taken_at_orig_tz = $photo->taken_at_orig_tz; $this->title = (Configs::getValueAsBool('file_name_hidden') && Auth::guest()) ? '' : $photo->title; $this->type = $photo->type; $this->updated_at = $photo->updated_at->toIso8601String(); - $this->rights = new PhotoRightsResource($photo); + $this->rights = new PhotoRightsResource($album); $this->next_photo_id = null; $this->previous_photo_id = null; $this->preformatted = new PreformattedPhotoData($photo, $this->size_variants->original); @@ -112,10 +113,10 @@ public function __construct(Photo $photo) } } - public static function fromModel(Photo $photo): PhotoResource - { - return new self($photo); - } + // public static function fromModel(Photo $photo): PhotoResource + // { + // return new self($photo); + // } private function setLocation(Photo $photo): void { diff --git a/app/Http/Resources/Models/SizeVariantsResouce.php b/app/Http/Resources/Models/SizeVariantsResouce.php index 86e2a15f71e..36ada3abb00 100644 --- a/app/Http/Resources/Models/SizeVariantsResouce.php +++ b/app/Http/Resources/Models/SizeVariantsResouce.php @@ -8,9 +8,10 @@ namespace App\Http\Resources\Models; +use App\Contracts\Models\AbstractAlbum; use App\Enum\SizeVariantType; use App\Models\Photo; -use App\Policies\PhotoPolicy; +use App\Policies\AlbumPolicy; use Illuminate\Support\Facades\Gate; use Spatie\LaravelData\Data; use Spatie\TypeScriptTransformer\Attributes\TypeScript; @@ -27,10 +28,10 @@ class SizeVariantsResouce extends Data public ?SizeVariantResource $thumb; public ?SizeVariantResource $placeholder; - public function __construct(Photo $photo) + public function __construct(Photo $photo, ?AbstractAlbum $album) { $size_variants = $photo->relationLoaded('size_variants') ? $photo->size_variants : null; - $downgrade = !Gate::check(PhotoPolicy::CAN_ACCESS_FULL_PHOTO, [Photo::class, $photo]) && + $downgrade = !Gate::check(AlbumPolicy::CAN_ACCESS_FULL_PHOTO, [AbstractAlbum::class, $album]) && !$photo->isVideo() && $size_variants?->hasMedium() === true; diff --git a/app/Http/Resources/Models/SmartAlbumResource.php b/app/Http/Resources/Models/SmartAlbumResource.php index 30ade0ec97c..81bfbec96d4 100644 --- a/app/Http/Resources/Models/SmartAlbumResource.php +++ b/app/Http/Resources/Models/SmartAlbumResource.php @@ -44,7 +44,7 @@ public function __construct(BaseSmartAlbum $smart_album) $this->id = $smart_album->get_id(); $this->title = $smart_album->get_title(); /** @disregard P1006 */ - $this->photos = $smart_album->relationLoaded('photos') ? PhotoResource::collect($smart_album->getPhotos()) : null; + $this->photos = $smart_album->relationLoaded('photos') ? $this->toPhotoResources($smart_album->getPhotos(), $smart_album) : null; $this->prepPhotosCollection(); if ($this->photos !== null) { // Prep collection with first and last link + which id is next. diff --git a/app/Http/Resources/Models/TagAlbumResource.php b/app/Http/Resources/Models/TagAlbumResource.php index c1d09a63627..559da172c3a 100644 --- a/app/Http/Resources/Models/TagAlbumResource.php +++ b/app/Http/Resources/Models/TagAlbumResource.php @@ -68,7 +68,7 @@ public function __construct(TagAlbum $tag_album) $this->copyright = $tag_album->copyright; // children - $this->photos = $tag_album->relationLoaded('photos') ? PhotoResource::collect($tag_album->photos) : null; + $this->photos = $tag_album->relationLoaded('photos') ? $this->toPhotoResources($tag_album->photos, $tag_album) : null; if ($this->photos !== null) { // Prep collection with first and last link + which id is next. $this->prepPhotosCollection(); diff --git a/app/Http/Resources/Rights/PhotoRightsResource.php b/app/Http/Resources/Rights/PhotoRightsResource.php index 4d0efa49bee..631026b9a0f 100644 --- a/app/Http/Resources/Rights/PhotoRightsResource.php +++ b/app/Http/Resources/Rights/PhotoRightsResource.php @@ -8,8 +8,9 @@ namespace App\Http\Resources\Rights; +use App\Contracts\Models\AbstractAlbum; use App\Models\Photo; -use App\Policies\PhotoPolicy; +use App\Policies\AlbumPolicy; use Illuminate\Support\Facades\Gate; use Spatie\LaravelData\Data; use Spatie\TypeScriptTransformer\Attributes\TypeScript; @@ -24,14 +25,14 @@ class PhotoRightsResource extends Data /** * Given a photo, returns the access rights associated to it. * - * @param Photo $photo + * @param ?AbstractAlbum $album * * @return void */ - public function __construct(Photo $photo) + public function __construct(?AbstractAlbum $album) { - $this->can_edit = Gate::check(PhotoPolicy::CAN_EDIT, [Photo::class, $photo]); - $this->can_download = Gate::check(PhotoPolicy::CAN_DOWNLOAD, [Photo::class, $photo]); - $this->can_access_full_photo = Gate::check(PhotoPolicy::CAN_ACCESS_FULL_PHOTO, [Photo::class, $photo]); + $this->can_edit = Gate::check(AlbumPolicy::CAN_EDIT, [AbstractAlbum::class, $album]); + $this->can_download = Gate::check(AlbumPolicy::CAN_DOWNLOAD, [AbstractAlbum::class, $album]); + $this->can_access_full_photo = Gate::check(AlbumPolicy::CAN_ACCESS_FULL_PHOTO, [AbstractAlbum::class, $album]); } } \ No newline at end of file diff --git a/app/Http/Resources/Traits/HasPrepPhotoCollection.php b/app/Http/Resources/Traits/HasPrepPhotoCollection.php index 0c5c74cadcd..35dcf317bca 100644 --- a/app/Http/Resources/Traits/HasPrepPhotoCollection.php +++ b/app/Http/Resources/Traits/HasPrepPhotoCollection.php @@ -8,6 +8,7 @@ namespace App\Http\Resources\Traits; +use App\Contracts\Models\AbstractAlbum; use App\Http\Resources\Models\PhotoResource; use App\Models\Configs; use Illuminate\Support\Collection; @@ -17,6 +18,11 @@ */ trait HasPrepPhotoCollection { + private function toPhotoResources(Collection $photos, ?AbstractAlbum $album): Collection + { + return $photos->map(fn ($photo) => new PhotoResource($photo, $album)); + } + private function prepPhotosCollection(): void { $previous_photo = null; diff --git a/app/Models/Album.php b/app/Models/Album.php index e28c97e490d..ab74c88911a 100644 --- a/app/Models/Album.php +++ b/app/Models/Album.php @@ -339,14 +339,15 @@ public function fixOwnershipOfChildren(): void ->whereBetween('albums._lft', [$lft + 1, $rgt - 1]); }) ->update(['owner_id' => $this->owner_id]); - Photo::query() - ->whereExists(function (BaseBuilder $q) use ($lft, $rgt): void { - $q - ->from('albums') - ->whereColumn('photos.album_id', '=', 'albums.id') - ->whereBetween('albums._lft', [$lft, $rgt]); - }) - ->update(['owner_id' => $this->owner_id]); + // TODO: FIX ME This is no longer correct. + // Photo::query() + // ->whereExists(function (BaseBuilder $q) use ($lft, $rgt): void { + // $q + // ->from('albums') + // ->whereColumn('photos.album_id', '=', 'albums.id') // ! column no longer exists! + // ->whereBetween('albums._lft', [$lft, $rgt]); + // }) + // ->update(['owner_id' => $this->owner_id]); } /** diff --git a/app/Models/Builders/AlbumBuilder.php b/app/Models/Builders/AlbumBuilder.php index 185627aff2c..f9313841956 100644 --- a/app/Models/Builders/AlbumBuilder.php +++ b/app/Models/Builders/AlbumBuilder.php @@ -9,6 +9,7 @@ namespace App\Models\Builders; use App\Constants\AccessPermissionConstants as APC; +use App\Constants\PhotoAlbum as PA; use App\Contracts\Exceptions\InternalLycheeException; use App\Eloquent\FixedQueryBuilderTrait; use App\Exceptions\Internal\QueryBuilderException; @@ -67,8 +68,9 @@ public function getModels($columns = ['*']): array ->whereColumn('a.parent_id', '=', 'albums.id'); $count_photos = DB::table('photos', 'p') + ->join(PA::PHOTO_ALBUM, 'p.id', '=', PA::PHOTO_ID) ->selectRaw('COUNT(*)') - ->whereColumn('p.album_id', '=', 'albums.id'); + ->whereColumn(PA::ALBUM_ID, '=', 'albums.id'); $this->addSelect([ 'min_taken_at' => $this->getTakenAtSQL()->selectRaw('MIN(taken_at)'), @@ -144,7 +146,8 @@ private function getTakenAtSQL(): Builder // use a non-Eloquent query here to avoid an infinite loop // with this query builder. return DB::table('albums', 'a') - ->join('photos', 'album_id', '=', 'a.id') + ->join(PA::PHOTO_ALBUM, 'a.id', '=', PA::ALBUM_ID) + ->join('photos', PA::PHOTO_ID, '=', 'photos.id') ->whereColumn('a._lft', '>=', 'albums._lft') ->whereColumn('a._rgt', '<=', 'albums._rgt') ->whereNotNull('taken_at'); @@ -220,14 +223,14 @@ private function applyVisibilityConditioOnPhotos(Builder $count_query, AlbumQuer $count_query->when($user_id !== null, fn ($q) => $album_query_policy->joinBaseAlbumOwnerId( query: $q, - second: 'p.album_id', + second: PA::ALBUM_ID, full: false, ) ); $album_query_policy->joinSubComputedAccessPermissions( query: $count_query, - second: 'p.album_id', + second: PA::ALBUM_ID, type: 'left', ); diff --git a/app/Models/Extensions/SizeVariants.php b/app/Models/Extensions/SizeVariants.php index 0bd1cd4df3a..94e61d007ab 100644 --- a/app/Models/Extensions/SizeVariants.php +++ b/app/Models/Extensions/SizeVariants.php @@ -263,41 +263,6 @@ public function deleteAll(): void (new Delete())->do(array_diff($ids, [null]))->do(); } - /** - * @throws ModelDBException - * @throws IllegalOrderOfOperationException - */ - public function replicate(Photo $duplicate_photo): SizeVariants - { - $duplicate = new SizeVariants($duplicate_photo); - self::replicateSizeVariant($duplicate, $this->original); - self::replicateSizeVariant($duplicate, $this->medium2x); - self::replicateSizeVariant($duplicate, $this->medium); - self::replicateSizeVariant($duplicate, $this->small2x); - self::replicateSizeVariant($duplicate, $this->small); - self::replicateSizeVariant($duplicate, $this->thumb2x); - self::replicateSizeVariant($duplicate, $this->thumb); - self::replicateSizeVariant($duplicate, $this->placeholder); - - return $duplicate; - } - - /** - * @throws ModelDBException - * @throws IllegalOrderOfOperationException - */ - private static function replicateSizeVariant(SizeVariants $duplicate, ?SizeVariant $size_variant): void - { - if ($size_variant !== null) { - $duplicate->create( - $size_variant->type, - $size_variant->short_path, - new ImageDimension($size_variant->width, $size_variant->height), - $size_variant->filesize - ); - } - } - /** * Returns true if at least one version of medium is not null. */ diff --git a/app/Models/Photo.php b/app/Models/Photo.php index 36b76084946..9ca940addcf 100644 --- a/app/Models/Photo.php +++ b/app/Models/Photo.php @@ -12,8 +12,10 @@ use App\Casts\ArrayCast; use App\Casts\DateTimeWithTimezoneCast; use App\Casts\MustNotSetCast; +use App\Constants\PhotoAlbum as PA; use App\Contracts\Models\HasUTCBasedTimes; use App\Enum\LicenseType; +use App\Enum\SmartAlbumType; use App\Enum\StorageDiskType; use App\Exceptions\Internal\IllegalOrderOfOperationException; use App\Exceptions\Internal\LycheeAssertionError; @@ -30,55 +32,57 @@ use App\Models\Extensions\ToArrayThrowsNotImplemented; use App\Models\Extensions\UTCBasedTimes; use App\Relations\HasManySizeVariants; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; +use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasOne; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Storage; use function Safe\preg_match; /** * App\Models\Photo. * - * @property string $id - * @property string $title - * @property string|null $description - * @property string[] $tags - * @property int $owner_id - * @property string|null $type - * @property string|null $iso - * @property string|null $aperture - * @property string|null $make - * @property string|null $model - * @property string|null $lens - * @property string|null $shutter - * @property string|null $focal - * @property float|null $latitude - * @property float|null $longitude - * @property float|null $altitude - * @property float|null $img_direction - * @property string|null $location - * @property Carbon|null $taken_at - * @property string|null $taken_at_orig_tz - * @property Carbon|null $initial_taken_at - * @property string|null $initial_taken_at_orig_tz - * @property bool $is_starred - * @property string|null $live_photo_short_path - * @property string|null $live_photo_url - * @property string|null $album_id - * @property string $checksum - * @property string $original_checksum - * @property LicenseType $license - * @property Carbon $created_at - * @property Carbon $updated_at - * @property string|null $live_photo_content_id - * @property string|null $live_photo_checksum - * @property Album|null $album - * @property User $owner - * @property SizeVariants $size_variants - * @property int $filesize - * @property Palette|null $palette + * @property string $id + * @property string $title + * @property string|null $description + * @property string[] $tags + * @property int $owner_id + * @property string|null $type + * @property string|null $iso + * @property string|null $aperture + * @property string|null $make + * @property string|null $model + * @property string|null $lens + * @property string|null $shutter + * @property string|null $focal + * @property float|null $latitude + * @property float|null $longitude + * @property float|null $altitude + * @property float|null $img_direction + * @property string|null $location + * @property Carbon|null $taken_at + * @property string|null $taken_at_orig_tz + * @property Carbon|null $initial_taken_at + * @property string|null $initial_taken_at_orig_tz + * @property bool $is_starred + * @property string|null $live_photo_short_path + * @property string|null $live_photo_url + * @property string $checksum + * @property string $original_checksum + * @property LicenseType $license + * @property Carbon $created_at + * @property Carbon $updated_at + * @property string|null $live_photo_content_id + * @property string|null $live_photo_checksum + * @property Collection $albums + * @property User $owner + * @property SizeVariants $size_variants + * @property int $filesize + * @property Palette|null $palette * * @method static PhotoBuilder|Photo addSelect($column) * @method static PhotoBuilder|Photo join(string $table, string $first, string $operator = null, string $second = null, string $type = 'inner', string $where = false) @@ -184,11 +188,11 @@ public function newEloquentBuilder($query): PhotoBuilder /** * Return the relationship between a Photo and its Album. * - * @return BelongsTo + * @return BelongsToMany */ - public function album(): BelongsTo + public function albums(): BelongsToMany { - return $this->belongsTo(Album::class, 'album_id', 'id'); + return $this->belongsToMany(Album::class, PA::PHOTO_ALBUM, 'photo_id', 'album_id', 'id', 'id'); } /** @@ -307,9 +311,9 @@ protected function getLicenseAttribute(?string $license): LicenseType return LicenseType::from($license); } - if ($this->album_id !== null && $this->relationLoaded('album')) { - return $this->album->license; - } + // if ($this->album_id !== null && $this->relationLoaded('album')) { + // return $this->album->license; + // } return Configs::getValueAsEnum('default_license', LicenseType::class); } @@ -428,32 +432,6 @@ public function isRaw(): bool return !$this->isPhoto() && !$this->isVideo(); } - /** - * @param string[] $except - * //method.childParameterType (contravariance) - */ - public function replicate(?array $except = null): Photo - { - $duplicate = parent::replicate($except); - // A photo has the following relations: (parent) album, owner and - // size_variants. - // While the duplicate may keep the relation to the same album and - // each photo requires an individual set of size variants. - // Se we unset the relation and explicitly duplicate the size variants. - $duplicate->unsetRelation('size_variants'); - // save duplicate so that the photo gets an ID - $duplicate->save(); - - $are_size_variants_originally_loaded = $this->relationLoaded('size_variants'); - // Duplicate the size variants of this instance for the duplicate - $duplicated_size_variants = $this->size_variants->replicate($duplicate); - if ($are_size_variants_originally_loaded) { - $duplicate->setRelation('size_variants', $duplicated_size_variants); - } - - return $duplicate; - } - /** * {@inheritDoc} * @@ -462,7 +440,10 @@ public function replicate(?array $except = null): Photo */ protected function performDeleteOnModel(): void { - $file_deleter = (new Delete())->do([$this->id]); + // Delete all the links to the photo. + DB::table(PA::PHOTO_ALBUM)->where('photo_id', $this->id)->delete(); + // Clean up the files. + $file_deleter = (new Delete())->do([$this->id], SmartAlbumType::UNSORTED->value); $this->exists = false; $file_deleter->do(); } diff --git a/app/Policies/PhotoPolicy.php b/app/Policies/PhotoPolicy.php index 0f03f3f2777..d7c9ba4fc1f 100644 --- a/app/Policies/PhotoPolicy.php +++ b/app/Policies/PhotoPolicy.php @@ -8,14 +8,19 @@ namespace App\Policies; +use App\Constants\PhotoAlbum as PA; use App\Enum\MetricsAccess; use App\Exceptions\ConfigurationKeyMissingException; use App\Exceptions\Internal\FrameworkException; use App\Exceptions\Internal\QueryBuilderException; +use App\Exceptions\UnauthorizedException; +use App\Models\Album; use App\Models\Configs; use App\Models\Photo; use App\Models\User; use Illuminate\Contracts\Container\BindingResolutionException; +use Illuminate\Support\Collection; +use Illuminate\Support\Facades\DB; class PhotoPolicy extends BasePolicy { @@ -23,7 +28,7 @@ class PhotoPolicy extends BasePolicy public const CAN_SEE = 'canSee'; public const CAN_DOWNLOAD = 'canDownload'; - public const CAN_DELETE = 'canDelete'; + // public const CAN_DELETE = 'canDelete'; public const CAN_EDIT = 'canEdit'; public const CAN_EDIT_ID = 'canEditById'; public const CAN_ACCESS_FULL_PHOTO = 'canAccessFullPhoto'; @@ -57,6 +62,11 @@ private function isOwner(?User $user, Photo $photo): bool return $user !== null && $photo->owner_id === $user->id; } + private function hasAlbums(Photo $photo): bool + { + return $photo->albums !== null && !$photo->albums->isEmpty(); + } + /** * Defines whether the photo is visible to the current user. * @@ -71,7 +81,7 @@ public function canSee(?User $user, Photo $photo): bool return true; } - return $photo->album !== null && $this->album_policy->canAccess($user, $photo->album); + return $this->hasAlbums($photo) && $this->reduction($photo->albums, fn ($a) => $this->album_policy->canAccess($user, $a)); } /** @@ -88,7 +98,7 @@ public function canDownload(?User $user, Photo $photo): bool return true; } - return $this->canSee($user, $photo) && $this->album_policy->canDownload($user, $photo->album); + return $this->hasAlbums($photo) && $this->reduction($photo->albums, fn ($a) => $this->album_policy->canDownload($user, $a)); } /** @@ -112,7 +122,7 @@ public function canEdit(User $user, Photo $photo) return true; } - return $this->canSee($user, $photo) && $this->album_policy->canEdit($user, $photo->album); + return $this->hasAlbums($photo) && $this->reduction($photo->albums, fn ($a) => $this->album_policy->canEdit($user, $a)); } /** @@ -140,9 +150,9 @@ public function canEditById(User $user, array $photo_ids): bool return true; } - $parents_id = Photo::query() - ->select('album_id') - ->whereIn('id', $photo_ids) + $parents_id = DB::table(PA::PHOTO_ALBUM) + ->select(PA::ALBUM_ID) + ->whereIn(PA::PHOTO_ID, $photo_ids) ->groupBy('album_id') ->pluck('album_id')->all(); @@ -169,7 +179,7 @@ public function canAccessFullPhoto(?User $user, Photo $photo): bool return false; } - return $this->album_policy->canAccessFullPhoto($user, $photo->album); + return $this->hasAlbums($photo) && $this->reduction($photo->albums, fn ($a) => $this->album_policy->canAccessFullPhoto($user, $a)); } /** @@ -179,14 +189,17 @@ public function canAccessFullPhoto(?User $user, Photo $photo): bool * * @return bool */ - public function canDelete(User $user, Photo $photo) - { - if ($this->isOwner($user, $photo)) { - return true; - } + // public function canDelete(User $user, Photo $photo) + // { + // if ($this->isOwner($user, $photo)) { + // return true; + // } - return $this->canSee($user, $photo) && $this->album_policy->canDelete($user, $photo->album); - } + // // TODO: refactor me. + // throw new UnauthorizedException('You are not allowed to delete this photo (yet).'); + + // return $this->canSee($user, $photo) && $this->album_policy->canDelete($user, $photo->album); + // } /** * Checks whether the designated photos are deletable by the current user. @@ -216,20 +229,21 @@ public function canDeleteById(User $user, array $photo_ids): bool // If there are any photos which are not in albums at this point, we fail. if ( Photo::query() + ->leftJoin(PA::PHOTO_ALBUM, 'photos.id', '=', PA::PHOTO_ID) ->whereNull('album_id') - ->whereIn('id', $photo_ids) + ->whereIn('photos.id', $photo_ids) ->count() > 0 ) { return false; } - $parent_i_ds = Photo::query() - ->select('album_id') - ->whereIn('id', $photo_ids) - ->groupBy('album_id') + $parent_ids = DB::table(PA::PHOTO_ALBUM) + ->select(PA::ALBUM_ID) + ->whereIn(PA::PHOTO_ID, $photo_ids) + ->groupBy(PA::ALBUM_ID) ->pluck('album_id')->all(); - return $this->album_policy->canDeleteById($user, $parent_i_ds); + return $this->album_policy->canDeleteById($user, $parent_ids); } /** @@ -252,4 +266,18 @@ public function canReadMetrics(?User $user, Photo $photo): bool default => false, }; } + + /** + * @param Collection $albums + * @param \Closure(Album $album): bool $reducer + * + * @return bool + */ + private function reduction(Collection $albums, \Closure $reducer): bool + { + return $albums->reduce( + fn (bool $carry, Album $album) => $carry || $reducer($album), + false + ); + } } diff --git a/app/Policies/PhotoQueryPolicy.php b/app/Policies/PhotoQueryPolicy.php index ec01cd27e1e..8e28907bab8 100644 --- a/app/Policies/PhotoQueryPolicy.php +++ b/app/Policies/PhotoQueryPolicy.php @@ -8,6 +8,7 @@ namespace App\Policies; +use App\Constants\PhotoAlbum as PA; use App\Contracts\Exceptions\InternalLycheeException; use App\Eloquent\FixedQueryBuilder; use App\Exceptions\Internal\InvalidQueryModelException; @@ -16,6 +17,7 @@ use App\Models\Photo; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Query\Builder as BaseBuilder; +use Illuminate\Database\Query\JoinClause; use Illuminate\Support\Facades\Auth; class PhotoQueryPolicy @@ -206,7 +208,7 @@ public function appendSearchabilityConditions(BaseBuilder $query, int|string|nul // root album, they must only see their own photos or public // photos (this is different to any other album: if users are // allowed to access an album, they may also see its content) - $query->whereNotNull('photos.album_id'); + // $query->whereNotNull(PA::ALBUM_ID); if ($user_id !== null) { $query->orWhere('photos.owner_id', '=', $user_id); @@ -261,7 +263,7 @@ private function appendSensitivityConditions(BaseBuilder $query, int|string|null // allowed to access an album, they may also see its content) $query->orWhere( fn ($q) => $q - ->whereNull('photos.album_id') + ->whereNull(PA::ALBUM_ID) ->where('photos.owner_id', '=', $user_id) ); } catch (\Throwable $e) { @@ -283,7 +285,8 @@ private function appendSensitivityConditions(BaseBuilder $query, int|string|null private function prepareModelQueryOrFail(FixedQueryBuilder $query, bool $add_albums, bool $add_base_albums): void { $model = $query->getModel(); - $table = $query->getQuery()->from; + $base_query = $query->getQuery(); + $table = $base_query->from; if (!($model instanceof Photo && $table === 'photos')) { throw new InvalidQueryModelException('photo'); } @@ -292,29 +295,44 @@ private function prepareModelQueryOrFail(FixedQueryBuilder $query, bool $add_alb // if no specific columns are yet set. // Otherwise, we cannot add a JOIN clause below // without accidentally adding all columns of the join, too. - $base_query = $query->getQuery(); + if ($base_query->columns === null || count($base_query->columns) === 0) { $query->select(['photos.*']); } + + /** @var array $joins */ + $joins = $base_query->joins ?? []; + + // Check if the photo album is already joined + $is_photo_album_joined = array_reduce($joins, fn (bool $is_photo_album_joined, JoinClause $join) => $is_photo_album_joined || PA::isJoinedToPhoto($join), false); + + if (!$is_photo_album_joined) { + $query->leftJoin( + table: PA::PHOTO_ALBUM, + first: 'photos.id', + operator: '=', + second: PA::PHOTO_ID); + } + if ($add_albums) { $query->leftJoin( table: 'albums', first: 'albums.id', operator: '=', - second: 'photos.album_id'); + second: PA::ALBUM_ID); } if ($add_base_albums) { $query->leftJoin( table: 'base_albums', first: 'base_albums.id', operator: '=', - second: 'photos.album_id'); + second: PA::ALBUM_ID); } // Necessary to apply the visibiliy/search conditions $this->album_query_policy->joinSubComputedAccessPermissions( query: $query, - second: 'photos.album_id' + second: PA::ALBUM_ID ); } } \ No newline at end of file diff --git a/app/Relations/BaseHasManyPhotos.php b/app/Relations/BaseHasManyPhotos.php index cdae629a060..505b45fd4f8 100644 --- a/app/Relations/BaseHasManyPhotos.php +++ b/app/Relations/BaseHasManyPhotos.php @@ -65,7 +65,7 @@ public function __construct(TagAlbum|Album $owning_album) // indirect condition. // Hence, the actually owning albums of the photos are not // necessarily loaded. - Photo::query()->with(['album', 'size_variants', 'palette']), + Photo::query()->with(['albums', 'size_variants', 'palette']), $owning_album ); } diff --git a/app/Relations/HasAlbumThumb.php b/app/Relations/HasAlbumThumb.php index 560e82ed0ef..06b5c61b02d 100644 --- a/app/Relations/HasAlbumThumb.php +++ b/app/Relations/HasAlbumThumb.php @@ -8,6 +8,7 @@ namespace App\Relations; +use App\Constants\PhotoAlbum as PA; use App\DTO\PhotoSortingCriterion; use App\Eloquent\FixedQueryBuilder; use App\Enum\ColumnSortingPhotoType; @@ -23,6 +24,7 @@ use Illuminate\Database\Eloquent\Relations\Relation; use Illuminate\Database\Query\Builder as BaseBuilder; use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Gate; /** @@ -48,7 +50,8 @@ public function __construct(Album $parent) $this->photo_query_policy = resolve(PhotoQueryPolicy::class); $this->sorting = PhotoSortingCriterion::createDefault(); parent::__construct( - Photo::query()->with(['size_variants' => (fn ($r) => Thumb::sizeVariantsFilter($r))]), + Photo::query() + ->with(['size_variants' => (fn ($r) => Thumb::sizeVariantsFilter($r))]), $parent ); } @@ -187,9 +190,10 @@ public function addEagerConstraints(array $models): void ->map(fn (Album $album) => $album->getKey()) ->values(); - $best_photo_id_select = Photo::query() - ->select(['photos.id AS photo_id']) - ->join('albums', 'albums.id', '=', 'photos.album_id') + $best_photo_id_select = DB::table(PA::PHOTO_ALBUM) + ->select(PA::PHOTO_ID) + ->join('photos', 'photos.id', '=', PA::PHOTO_ID) + ->join('albums', 'albums.id', '=', PA::ALBUM_ID) ->whereColumn('albums._lft', '>=', 'covered_albums._lft') ->whereColumn('albums._rgt', '<=', 'covered_albums._rgt') ->orderBy('photos.' . ColumnSortingPhotoType::IS_STARRED->value, OrderSortingType::DESC->value) @@ -197,9 +201,9 @@ public function addEagerConstraints(array $models): void ->limit(1); if (Auth::user()?->may_administrate !== true) { - $best_photo_id_select->where(function (Builder $query2): void { + $best_photo_id_select->where(function (BaseBuilder $query2): void { $this->photo_query_policy->appendSearchabilityConditions( - $query2->getQuery(), + $query2, 'covered_albums._lft', 'covered_albums._rgt' ); diff --git a/app/Relations/HasManyChildPhotos.php b/app/Relations/HasManyChildPhotos.php index 91f1d7f6912..726600aaefd 100644 --- a/app/Relations/HasManyChildPhotos.php +++ b/app/Relations/HasManyChildPhotos.php @@ -8,6 +8,7 @@ namespace App\Relations; +use App\Constants\PhotoAlbum as PA; use App\Contracts\Exceptions\InternalLycheeException; use App\Eloquent\FixedQueryBuilder; use App\Enum\OrderSortingType; @@ -18,12 +19,12 @@ use App\Policies\PhotoQueryPolicy; use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\InvalidCastException; -use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Relations\BelongsToMany; /** - * @extends HasManyBidirectionally + * @extends BelongsToMany */ -class HasManyChildPhotos extends HasManyBidirectionally +class HasManyChildPhotos extends BelongsToMany { protected PhotoQueryPolicy $photo_query_policy; @@ -35,11 +36,14 @@ public function __construct(Album $owning_album) // attributes must be initialized by then $this->photo_query_policy = resolve(PhotoQueryPolicy::class); parent::__construct( - Photo::query(), - $owning_album, - 'album_id', - 'id', - 'album' + query: Photo::query(), + parent: $owning_album, + table: PA::PHOTO_ALBUM, + foreignPivotKey: PA::ALBUM_ID, + relatedPivotKey: PA::PHOTO_ID, + parentKey: 'id', + relatedKey: 'id', + relationName: 'albums' ); } @@ -73,8 +77,10 @@ public function getParent(): Album */ public function addConstraints() { + $this->performJoin(); + if (static::$constraints) { - parent::addConstraints(); + $this->addWhereConstraints(); $this->photo_query_policy->applyVisibilityFilter($this->getRelationQuery()); } } @@ -94,10 +100,12 @@ public function addEagerConstraints(array $models) * @return Collection * * @throws InvalidOrderDirectionException + * + * @phpstan-ignore method.childReturnType */ public function getResults(): Collection { - if (is_null($this->getParentKey())) { + if (is_null($this->parent->{$this->parentKey})) { // @phpstan-ignore property.dynamicName return $this->related->newCollection(); } @@ -135,9 +143,10 @@ public function match(array $models, Collection $results, $relation): array // matching very convenient and easy work. Then we'll just return them. /** @var Album $model */ foreach ($models as $model) { - if (isset($dictionary[$key = $this->getDictionaryKey($model->getAttribute($this->localKey))])) { - /** @var Collection $children_of_model */ - $children_of_model = $this->getRelationValue($dictionary, $key, 'many'); + $key = $this->getDictionaryKey($model->{$this->parentKey}); // @phpstan-ignore property.dynamicName + + if (isset($dictionary[$key])) { + $children_of_model = $this->related->newCollection($dictionary[$key]); $sorting = $model->getEffectivePhotoSorting(); $children_of_model = $children_of_model ->sortBy( @@ -146,15 +155,34 @@ public function match(array $models, Collection $results, $relation): array $sorting->order === OrderSortingType::DESC ) ->values(); - $model->setRelation($relation, $children_of_model); - // This is the newly added code which sets this method apart - // from the original method and additionally sets the - // reverse link - - foreach ($children_of_model as $child_model) { - $child_model->setRelation($this->foreign_method_name, $model); - } + + $model->setRelation( + $relation, $children_of_model + ); } + + // if (isset($dictionary[$key])) { + + // // if (isset($dictionary[$key = $this->getDictionaryKey($model->getAttribute($this->localKey))])) { + // /** @var Collection $children_of_model */ + // $children_of_model = $this->getRelationValue($dictionary, $key, 'many'); + // $sorting = $model->getEffectivePhotoSorting(); + // $children_of_model = $children_of_model + // ->sortBy( + // $sorting->column->value, + // in_array($sorting->column, SortingDecorator::POSTPONE_COLUMNS, true) ? SORT_NATURAL | SORT_FLAG_CASE : SORT_REGULAR, + // $sorting->order === OrderSortingType::DESC + // ) + // ->values(); + // $model->setRelation($relation, $children_of_model); + // // This is the newly added code which sets this method apart + // // from the original method and additionally sets the + // // reverse link + + // // foreach ($children_of_model as $child_model) { + // // $child_model->setRelation($this->foreign_method_name, $model); + // // } + // } } return $models; diff --git a/app/SmartAlbums/BaseSmartAlbum.php b/app/SmartAlbums/BaseSmartAlbum.php index 35845f0ac76..900a180083d 100644 --- a/app/SmartAlbums/BaseSmartAlbum.php +++ b/app/SmartAlbums/BaseSmartAlbum.php @@ -8,6 +8,7 @@ namespace App\SmartAlbums; +use App\Constants\PhotoAlbum as PA; use App\Contracts\Exceptions\InternalLycheeException; use App\Contracts\Models\AbstractAlbum; use App\DTO\PhotoSortingCriterion; @@ -113,7 +114,7 @@ public function get_photos(): Collection */ public function photos(): Builder { - $base_query = Photo::query()->with(['album', 'size_variants', 'statistics', 'palette']); + $base_query = Photo::query()->leftJoin(PA::PHOTO_ALBUM, 'photos.id', '=', PA::PHOTO_ID)->with(['size_variants', 'statistics', 'palette']); if (!Configs::getValueAsBool('SA_override_visibility')) { return $this->photo_query_policy diff --git a/app/SmartAlbums/UnsortedAlbum.php b/app/SmartAlbums/UnsortedAlbum.php index b44ce46e5ad..978e31387a0 100644 --- a/app/SmartAlbums/UnsortedAlbum.php +++ b/app/SmartAlbums/UnsortedAlbum.php @@ -8,6 +8,7 @@ namespace App\SmartAlbums; +use App\Constants\PhotoAlbum as PA; use App\Enum\SmartAlbumType; use App\Exceptions\ConfigurationKeyMissingException; use App\Exceptions\Internal\FrameworkException; @@ -27,7 +28,7 @@ public function __construct() { parent::__construct( id: SmartAlbumType::UNSORTED, - smart_condition: fn (Builder $q) => $q->whereNull('photos.album_id') + smart_condition: fn (Builder $q) => $q->whereNull(PA::ALBUM_ID) ); } @@ -45,7 +46,7 @@ public static function getInstance(): self public function photos(): Builder { if ($this->public_permissions !== null) { - return Photo::query()->with(['album', 'size_variants', 'statistics', 'palette']) + return Photo::query()->leftJoin(PA::PHOTO_ALBUM, 'photos.id', '=', PA::PHOTO_ID)->with(['size_variants', 'statistics', 'palette']) ->where($this->smart_photo_condition); } diff --git a/database/factories/PhotoFactory.php b/database/factories/PhotoFactory.php index a194e11e040..6807712dc1e 100644 --- a/database/factories/PhotoFactory.php +++ b/database/factories/PhotoFactory.php @@ -58,7 +58,6 @@ public function definition(): array 'taken_at' => $now, 'initial_taken_at_orig_tz' => null, 'is_starred' => false, - 'album_id' => null, 'checksum' => sha1(rand()), 'original_checksum' => sha1(rand()), 'license' => 'none', @@ -126,12 +125,8 @@ public function with_tags(string $tags): self */ public function in(Album $album): self { - return $this->state(function (array $attributes) use ($album) { - return [ - 'album_id' => $album->id, - ]; - })->afterCreating(function (Photo $photo) { - $photo->load('album', 'owner'); + return $this->hasAttached([$album])->afterCreating(function (Photo $photo) { + $photo->load('albums', 'owner'); }); } diff --git a/database/migrations/2025_05_28_201707_create_photo_album_pivot.php b/database/migrations/2025_05_28_201707_create_photo_album_pivot.php new file mode 100644 index 00000000000..24395ba5f0a --- /dev/null +++ b/database/migrations/2025_05_28_201707_create_photo_album_pivot.php @@ -0,0 +1,49 @@ +char('album_id', self::RANDOM_ID_LENGTH)->index()->nullable(false); + $table->char('photo_id', self::RANDOM_ID_LENGTH)->index()->nullable(false); + $table->primary(['photo_id', 'album_id']); + $table->index(['album_id', 'photo_id']); + $table->foreign('photo_id')->references('id')->on('photos'); + $table->foreign('album_id')->references('id')->on('albums'); + }); + + DB::statement('INSERT INTO photo_album (photo_id, album_id) SELECT id, album_id FROM photos WHERE album_id IS NOT NULL'); + + Schema::table('photos', function (Blueprint $table) { $table->dropForeign(['album_id']); }); + Schema::table('photos', function (Blueprint $table) { $table->renameColumn('album_id', 'old_album_id'); }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::dropIfExists('photo_album'); + + Schema::table('photos', function (Blueprint $table) { + $table->renameColumn('old_album_id', 'album_id'); + $table->foreign('album_id')->references('id')->on('albums'); + }); + } +}; diff --git a/resources/js/components/forms/gallery-dialogs/DeleteDialog.vue b/resources/js/components/forms/gallery-dialogs/DeleteDialog.vue index 02d68206fe5..e7f355b4185 100644 --- a/resources/js/components/forms/gallery-dialogs/DeleteDialog.vue +++ b/resources/js/components/forms/gallery-dialogs/DeleteDialog.vue @@ -103,7 +103,7 @@ function executeDeletePhoto() { } else { photoDeletedIds = props.photoIds as string[]; } - PhotoService.delete(photoDeletedIds).then(() => { + PhotoService.delete(photoDeletedIds, getParentId() ?? "unsorted").then(() => { toast.add({ severity: "success", summary: trans("dialogs.photo_delete.deleted"), diff --git a/resources/js/components/forms/gallery-dialogs/MoveDialog.vue b/resources/js/components/forms/gallery-dialogs/MoveDialog.vue index 43a983f151a..5c3f79277dd 100644 --- a/resources/js/components/forms/gallery-dialogs/MoveDialog.vue +++ b/resources/js/components/forms/gallery-dialogs/MoveDialog.vue @@ -165,7 +165,7 @@ function executeMovePhoto() { } else { photoMovedIds = props.photoIds as string[]; } - PhotoService.move({ album_id: destination_id.value, photo_ids: photoMovedIds }).then(() => { + PhotoService.move({ from_id: getParentId() ?? null, album_id: destination_id.value, photo_ids: photoMovedIds }).then(() => { toast.add({ severity: "success", summary: sprintf(trans("dialogs.move_photo.moved"), titleMovedTo.value), diff --git a/resources/js/services/photo-service.ts b/resources/js/services/photo-service.ts index edbdc30ed5d..71fe5e27efb 100644 --- a/resources/js/services/photo-service.ts +++ b/resources/js/services/photo-service.ts @@ -13,6 +13,7 @@ export type PhotoUpdateRequest = { export type PhotoMove = { photo_ids: string[]; album_id: string | null; + from_id: string | null; }; const PhotoService = { @@ -44,8 +45,8 @@ const PhotoService = { return axios.post(`${Constants.getApiUrl()}Photo::copy`, { album_id: destination_id, photo_ids: photo_ids }); }, - delete(photo_ids: string[]): Promise { - return axios.delete(`${Constants.getApiUrl()}Photo`, { data: { photo_ids: photo_ids } }); + delete(photo_ids: string[], from_id: string): Promise { + return axios.delete(`${Constants.getApiUrl()}Photo`, { data: { photo_ids: photo_ids, from_id: from_id } }); }, star(photo_ids: string[], is_starred: boolean): Promise { diff --git a/tests/Feature_v2/Album/AssertableZipArchive.php b/tests/Feature_v2/Album/AssertableZipArchive.php index 3478d5213f7..c78bccf96b7 100644 --- a/tests/Feature_v2/Album/AssertableZipArchive.php +++ b/tests/Feature_v2/Album/AssertableZipArchive.php @@ -62,7 +62,7 @@ public static function createFromResponse(TestResponse $response): self * * @return void */ - public function assertContainsFile(string $fileName, ?int $expectedFileSize): void + public function assertContainsFile(string $fileName, ?int $expectedFileSize = null): void { $stat = $this->statName($fileName); PHPUnit::assertNotFalse($stat, 'Could not assert that ZIP archive contains ' . $fileName); diff --git a/tests/Feature_v2/Album/DownloadTest.php b/tests/Feature_v2/Album/DownloadTest.php index d758e6c7571..f121a275e5e 100644 --- a/tests/Feature_v2/Album/DownloadTest.php +++ b/tests/Feature_v2/Album/DownloadTest.php @@ -171,44 +171,40 @@ public function testAmbiguousPhotoDownload(): void $this->assertOk($response); $photo = $response->json('resource.photos.0'); - $this->postJson('Photo::copy', [ - 'photo_ids' => [$photo['id']], - 'album_id' => $this->album5->id, - ]); - $this->assertOk($response); - - $response = $this->getJsonWithData('Album', ['album_id' => $this->album5->id]); - $this->assertOk($response); - $response->assertJsonCount(2, 'resource.photos'); + // $this->postJson('Photo::copy', [ + // 'photo_ids' => [$photo['id']], + // 'album_id' => $this->album5->id, + // ]); + // $this->assertOk($response); $response = $this->actingAs($this->admin)->upload( 'Photo', filename: TestConstants::SAMPLE_FILE_NIGHT_IMAGE, album_id: $this->album5->id, - file_name: TestConstants::PHOTO_NIGHT_TITLE . '.jpg' - ); + file_name: TestConstants::PHOTO_NIGHT_TITLE . '.jpeg'); $this->assertCreated($response); + $response = $this->patchJson('Photo::rename', [ + 'photo_id' => $photo['id'], + 'title' => TestConstants::PHOTO_NIGHT_TITLE . '.jpeg', + ]); + $this->assertNoContent($response); + $response = $this->getJsonWithData('Album', ['album_id' => $this->album5->id]); $this->assertOk($response); - $response->assertJsonCount(3, 'resource.photos'); + $response->assertJsonCount(2, 'resource.photos'); $photoID1 = $response->json('resource.photos.0.id'); $photoID2a = $response->json('resource.photos.1.id'); - $photoID2b = $response->json('resource.photos.2.id'); - $photoArchiveResponse = $this->download( - photo_ids: [$photoID1, $photoID2a, $photoID2b], - from_id: $this->album5->id, - kind: DownloadVariantType::ORIGINAL - ); + $response->assertJsonPath('resource.photos.0.title', TestConstants::PHOTO_NIGHT_TITLE . '.jpeg'); + $response->assertJsonPath('resource.photos.1.title', TestConstants::PHOTO_NIGHT_TITLE . '.jpeg'); + + $photoArchiveResponse = $this->download([$photoID1, $photoID2a], kind: DownloadVariantType::ORIGINAL); $zipArchive = AssertableZipArchive::createFromResponse($photoArchiveResponse); - $zipArchive->assertContainsFilesExactly([ - 'night.jpg' => ['size' => filesize(base_path(TestConstants::SAMPLE_FILE_NIGHT_IMAGE))], - 'mongolia-1.jpeg' => ['size' => filesize(base_path(TestConstants::SAMPLE_FILE_MONGOLIA_IMAGE))], - 'mongolia-2.jpeg' => ['size' => filesize(base_path(TestConstants::SAMPLE_FILE_MONGOLIA_IMAGE))], - ]); + $zipArchive->assertContainsFile('night-1.jpeg'); + $zipArchive->assertContainsFile('night-2.jpeg'); } public function testPhotoDownloadWithMultiByteFilename(): void diff --git a/tests/Feature_v2/Commands/TakeDateTest.php b/tests/Feature_v2/Commands/TakeDateTest.php index a79b2d6afe2..f50ebc39746 100644 --- a/tests/Feature_v2/Commands/TakeDateTest.php +++ b/tests/Feature_v2/Commands/TakeDateTest.php @@ -38,6 +38,8 @@ public function testNoUpdateRequired(): void public function testSetUploadTimeFromFileTime(): void { // Make sure that the tables are empty before running this tests. + DB::table('photo_album')->delete(); + DB::table('statistics')->whereNotNull('photo_id')->delete(); DB::table('size_variants')->delete(); DB::table('photos')->delete(); diff --git a/tests/Feature_v2/Photo/PhotoAddTest.php b/tests/Feature_v2/Photo/PhotoAddTest.php index 4b709d8267b..29d243f059d 100644 --- a/tests/Feature_v2/Photo/PhotoAddTest.php +++ b/tests/Feature_v2/Photo/PhotoAddTest.php @@ -68,7 +68,7 @@ public function testAddPhotoAuthorizedOwner(): void ], ], ]); - $response = $this->deleteJson('Photo', ['photo_ids' => [$response->json('resource.photos.0.id')]]); + $response = $this->deleteJson('Photo', ['photo_ids' => [$response->json('resource.photos.0.id')], 'from_id' => 'unsorted']); $this->assertNoContent($response); $this->clearCachedSmartAlbums(); @@ -98,7 +98,7 @@ public function testDuplicate(): void self::assertNotEquals($id1, $id2); - $response = $this->deleteJson('Photo', ['photo_ids' => [$id1, $id2]]); + $response = $this->deleteJson('Photo', ['photo_ids' => [$id1, $id2], 'from_id' => 'unsorted']); $this->assertNoContent($response); $this->catchFailureSilence = ["App\Exceptions\MediaFileOperationException"]; @@ -162,7 +162,7 @@ public function testGoogleMotionPicture(): void $response = $this->getJsonWithData('Album', ['album_id' => 'unsorted']); $this->assertOk($response); $id = $response->json('resource.photos.0.id'); - $response = $this->deleteJson('Photo', ['photo_ids' => [$id]]); + $response = $this->deleteJson('Photo', ['photo_ids' => [$id], 'from_id' => 'unsorted']); $this->assertNoContent($response); } } diff --git a/tests/Feature_v2/Photo/PhotoDeleteTest.php b/tests/Feature_v2/Photo/PhotoDeleteTest.php index 5de976bc23a..ae9b9849110 100644 --- a/tests/Feature_v2/Photo/PhotoDeleteTest.php +++ b/tests/Feature_v2/Photo/PhotoDeleteTest.php @@ -29,13 +29,13 @@ public function testDeletePhotoUnauthorizedForbidden(): void $response = $this->deleteJson('Photo', [ 'photo_ids' => [$this->photo1->id], - 'album_id' => $this->album2->id, + 'from_id' => $this->album2->id, ]); $this->assertUnauthorized($response); $response = $this->actingAs($this->userNoUpload)->deleteJson('Photo', [ 'photo_ids' => [$this->photo1->id], - 'album_id' => $this->album2->id, + 'from_id' => $this->album2->id, ]); $this->assertForbidden($response); } @@ -47,6 +47,7 @@ public function testDeletePhotoAuthorizedOwner(): void $response = $this->actingAs($this->userLocked)->deleteJson('Photo', [ 'photo_ids' => [$this->photo1->id], + 'from_id' => $this->album1->id, ]); $this->assertForbidden($response); @@ -56,6 +57,7 @@ public function testDeletePhotoAuthorizedOwner(): void $response = $this->actingAs($this->userMayUpload1)->deleteJson('Photo', [ 'photo_ids' => [$this->photo1->id], + 'from_id' => $this->album1->id, ]); $this->assertNoContent($response); diff --git a/tests/Feature_v2/Photo/PhotoEditTest.php b/tests/Feature_v2/Photo/PhotoEditTest.php index 62836be5310..0f57f6561e8 100644 --- a/tests/Feature_v2/Photo/PhotoEditTest.php +++ b/tests/Feature_v2/Photo/PhotoEditTest.php @@ -36,6 +36,7 @@ public function testEditPhotoUnauthorizedForbiddenUnprocessable(): void 'license' => 'none', 'taken_at' => null, 'upload_date' => '2021-01-01', + 'from_id' => $this->album1->id, ]); $this->assertUnauthorized($response); @@ -47,6 +48,7 @@ public function testEditPhotoUnauthorizedForbiddenUnprocessable(): void 'license' => 'none', 'taken_at' => null, 'upload_date' => '2021-01-01', + 'from_id' => $this->album1->id, ]); $this->assertForbidden($response); @@ -64,6 +66,7 @@ public function testEditPhotoAuthorizedOwner(): void 'license' => 'none', 'taken_at' => null, 'upload_date' => '2021-01-01', + 'from_id' => $this->album1->id, ]); $this->assertOk($response); @@ -89,6 +92,7 @@ public function testEditPhotoAuthorizedOwner(): void 'license' => 'none', 'taken_at' => '2021-01-01T00:00:00+00:00', 'upload_date' => '2021-01-01', + 'from_id' => $this->album1->id, ]); $this->assertOk($response); @@ -115,6 +119,7 @@ public function testEditPhotoAuthorizedOwner(): void 'license' => 'none', 'taken_at' => null, 'upload_date' => '2021-01-01', + 'from_id' => $this->album1->id, ]); $this->assertOk($response); @@ -145,6 +150,7 @@ public function testEditPhotoAuthorizedOwnerNullTakenDate(): void 'license' => 'none', 'taken_at' => null, 'upload_date' => '2021-01-01', + 'from_id' => $this->album1->id, ]); $this->assertOk($response); @@ -171,6 +177,7 @@ public function testEditPhotoAuthorizedOwnerNullTakenDate(): void 'license' => 'none', 'taken_at' => '2021-01-01T00:00:00+00:00', 'upload_date' => '2021-01-01', + 'from_id' => $this->album1->id, ]); $this->assertOk($response); @@ -197,6 +204,7 @@ public function testEditPhotoAuthorizedOwnerNullTakenDate(): void 'license' => 'none', 'taken_at' => null, 'upload_date' => '2021-01-01', + 'from_id' => $this->album1->id, ]); $this->assertOk($response); diff --git a/tests/Feature_v2/Photo/PhotoMoveTest.php b/tests/Feature_v2/Photo/PhotoMoveTest.php index a9faaeb5a3d..e51d9a1a2f8 100644 --- a/tests/Feature_v2/Photo/PhotoMoveTest.php +++ b/tests/Feature_v2/Photo/PhotoMoveTest.php @@ -28,12 +28,14 @@ public function testMovePhotoUnauthorizedForbidden(): void $this->assertUnprocessable($response); $response = $this->postJson('Photo::move', [ + 'from_id' => $this->album1->id, 'photo_ids' => [$this->photo1->id], 'album_id' => $this->album2->id, ]); $this->assertUnauthorized($response); $response = $this->actingAs($this->userNoUpload)->postJson('Photo::move', [ + 'from_id' => $this->album1->id, 'photo_ids' => [$this->photo1->id], 'album_id' => $this->album2->id, ]); @@ -46,6 +48,7 @@ public function testMovePhotoAuthorizedOwner(): void $this->assertUnprocessable($response); $response = $this->actingAs($this->userMayUpload1)->postJson('Photo::move', [ + 'from_id' => $this->album1->id, 'photo_ids' => [$this->photo1->id], 'album_id' => $this->album2->id, ]); @@ -56,6 +59,7 @@ public function testMovePhotoAuthorizedOwner(): void $response->assertJsonCount(2, 'resource.photos'); $response = $this->actingAs($this->userMayUpload1)->postJson('Photo::move', [ + 'from_id' => $this->album1->id, 'photo_ids' => [$this->photo1->id], 'album_id' => $this->subAlbum1->id, ]); @@ -78,4 +82,43 @@ public function testMovePhotoAuthorizedOwner(): void ], ]); } + + public function testMovePhotoAuthorizedOwnerUnosorted(): void + { + $this->clearCachedSmartAlbums(); + $response = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album', ['album_id' => 'unsorted']); + $this->assertOk($response); + $response->assertJsonCount(1, 'resource.photos'); + $response->assertDontSee($this->photo1->id); + + $response = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album', ['album_id' => $this->album1->id]); + $this->assertOk($response); + $response->assertJsonCount(2, 'resource.photos'); + $response->assertSee($this->photo1->id); + + $response = $this->actingAs($this->userMayUpload1)->postJson('Photo::move', [ + 'from_id' => $this->album1->id, + 'photo_ids' => [$this->photo1->id], + 'album_id' => null, + ]); + $this->assertNoContent($response); + + $response = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album', ['album_id' => $this->album1->id]); + $this->assertOk($response); + $response->assertJsonCount(1, 'resource.photos'); + $response->assertDontSee($this->photo1->id); + + $this->clearCachedSmartAlbums(); + $response = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album', ['album_id' => 'unsorted']); + $this->assertOk($response); + $response->assertJsonCount(2, 'resource.photos'); + $response->assertSee($this->photo1->id); + + $response = $this->actingAs($this->userMayUpload1)->postJson('Photo::move', [ + 'from_id' => 'unsorted', + 'photo_ids' => [$this->photo1->id], + 'album_id' => $this->album1->id, + ]); + $this->assertNoContent($response); + } } \ No newline at end of file diff --git a/tests/Traits/CatchFailures.php b/tests/Traits/CatchFailures.php index 78591edbbe6..64d23fe5b16 100644 --- a/tests/Traits/CatchFailures.php +++ b/tests/Traits/CatchFailures.php @@ -30,10 +30,24 @@ trait CatchFailures /** * Some of the exceptions we get are expected. We silence then. * - * @var array + * @var string[] */ protected array $catchFailureSilence = ["App\Exceptions\MediaFileOperationException"]; + /** + * We trim the trace of exceptions to get better data. + * + * @var string[] + */ + protected array $exception_noise = [ + 'Illuminate\Database\Query\Builder', + 'Illuminate\Database\Connection', + 'Illuminate\Pipeline\Pipeline', + 'Illuminate\Container\BoundMethod', + 'Illuminate\Container\Util', + 'Illuminate\Container\Container', + ]; + /** * @param TestResponse<\Illuminate\Http\JsonResponse> $response * @param int|array $expectedStatusCode @@ -42,6 +56,8 @@ trait CatchFailures */ protected function assertStatus(TestResponse $response, int|array $expectedStatusCode): void { + $expectedStatusCodeArray = is_int($expectedStatusCode) ? [$expectedStatusCode] : $expectedStatusCode; + if ($response->getStatusCode() === 500 && $expectedStatusCode !== 500) { $exception = $response->json(); if (in_array($exception['exception'], $this->catchFailureSilence, true)) { @@ -49,12 +65,7 @@ protected function assertStatus(TestResponse $response, int|array $expectedStatu } $this->trimException($exception); dump($exception); - } - $expectedStatusCodeArray = is_int($expectedStatusCode) ? [$expectedStatusCode] : $expectedStatusCode; - - // We remove 204 as it does not have content - // We remove 302 because it does not have json data. - if (!in_array($response->getStatusCode(), [204, 302, ...$expectedStatusCodeArray], true)) { + } elseif (!in_array($response->getStatusCode(), [204, 302, ...$expectedStatusCodeArray], true)) { $exception = $response->json(); $this->trimException($exception); dump($exception); @@ -80,6 +91,7 @@ protected function assertStatus(TestResponse $response, int|array $expectedStatu private function trimException(array &$exception): void { if (isset($exception['trace'])) { + $exception['trace'] = array_values(array_filter($exception['trace'], fn ($t) => !in_array($t['class'] ?? '', $this->exception_noise, true))); $exception['trace'] = array_slice($exception['trace'], 0, 3); } diff --git a/tests/Traits/RequiresEmptyPhotos.php b/tests/Traits/RequiresEmptyPhotos.php index 58b45065a66..b1a942a4c15 100644 --- a/tests/Traits/RequiresEmptyPhotos.php +++ b/tests/Traits/RequiresEmptyPhotos.php @@ -18,6 +18,7 @@ namespace Tests\Traits; +use App\Constants\PhotoAlbum as PA; use Illuminate\Support\Facades\DB; use function Safe\fileowner; use function Safe\scandir; @@ -33,6 +34,7 @@ protected function setUpRequiresEmptyPhotos(): void { $this->setUpInteractsWithFilesystemPermissions(); // Assert that photo table is empty + $this->assertDatabaseCount(PA::PHOTO_ALBUM, 0); $this->assertDatabaseCount('size_variants', 0); $this->assertDatabaseCount('photos', 0); $this->assertDatabaseCount('jobs_history', 0); @@ -46,6 +48,7 @@ protected function setUpRequiresEmptyPhotos(): void protected function tearDownRequiresEmptyPhotos(): void { // Clean up remaining stuff from tests + DB::table(PA::PHOTO_ALBUM)->delete(); DB::table('size_variants')->delete(); DB::table('photos')->delete(); DB::table('jobs_history')->delete(); diff --git a/tests/Unit/Http/Requests/Album/DeleteAlbumsRequestTest.php b/tests/Unit/Http/Requests/Album/DeleteAlbumsRequestTest.php index 8228fbdd640..9bf1e28fa1e 100644 --- a/tests/Unit/Http/Requests/Album/DeleteAlbumsRequestTest.php +++ b/tests/Unit/Http/Requests/Album/DeleteAlbumsRequestTest.php @@ -14,7 +14,7 @@ use App\Contracts\Models\AbstractAlbum; use App\Http\Requests\Album\DeleteAlbumsRequest; use App\Policies\AlbumPolicy; -use App\Rules\AlbumIDRule; +use App\Rules\RandomIDRule; use Illuminate\Support\Facades\Gate; use Tests\Unit\Http\Requests\Base\BaseRequestTest; @@ -40,7 +40,7 @@ public function testRules(): void $expectedRuleMap = [ RequestAttribute::ALBUM_IDS_ATTRIBUTE => 'required|array|min:1', - RequestAttribute::ALBUM_IDS_ATTRIBUTE . '.*' => ['required', new AlbumIDRule(false)], + RequestAttribute::ALBUM_IDS_ATTRIBUTE . '.*' => ['required', new RandomIDRule(false)], ]; $this->assertCount(count($expectedRuleMap), $rules);