Skip to content

Commit c2fee5f

Browse files
authored
Fix downloading image directly from smart album (#4005)
1 parent 2fa1ce7 commit c2fee5f

File tree

3 files changed

+66
-20
lines changed

3 files changed

+66
-20
lines changed

app/Actions/Album/BaseArchive.php

Lines changed: 62 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use Composer\InstalledVersions;
2626
use Composer\Semver\VersionParser;
2727
use Illuminate\Database\Eloquent\Model;
28+
use Illuminate\Pagination\LengthAwarePaginator;
2829
use Illuminate\Support\Collection;
2930
use Illuminate\Support\Facades\Gate;
3031
use Safe\Exceptions\InfoException;
@@ -229,6 +230,67 @@ private function compressAlbum(AbstractAlbum $album, array &$used_dir_names, ?st
229230
// TODO: Ensure that the size variant `original` for each photo is eagerly loaded as it is needed below. This must be solved in close coordination with `ArchiveAlbumRequest`.
230231
$photos = $album->get_photos();
231232

233+
// For smart albums, get_photos() returns a paginator. We need to iterate through all pages.
234+
if ($photos instanceof LengthAwarePaginator) {
235+
$this->compressPhotosFromPaginator($photos, $album, $full_name_of_directory, $used_file_names, $zip);
236+
} else {
237+
$this->compressPhotosFromCollection($photos, $album, $full_name_of_directory, $used_file_names, $zip);
238+
}
239+
240+
// Recursively compress sub-albums
241+
if ($album instanceof Album) {
242+
$sub_dirs = [];
243+
// TODO: For higher efficiency, ensure that the photos of each child album together with the original size variant are eagerly loaded.
244+
$sub_albums = $album->children;
245+
foreach ($sub_albums as $sub_album) {
246+
try {
247+
$this->compressAlbum($sub_album, $sub_dirs, $full_name_of_directory, $zip);
248+
// @codeCoverageIgnoreStart
249+
} catch (\Throwable $e) {
250+
Handler::reportSafely($e);
251+
}
252+
// @codeCoverageIgnoreEnd
253+
}
254+
}
255+
}
256+
257+
/**
258+
* Compresses photos from a paginator by iterating through all pages.
259+
*
260+
* @param LengthAwarePaginator<int,Photo> $paginator
261+
* @param AbstractAlbum $album
262+
* @param string $full_name_of_directory
263+
* @param array<string> $used_file_names
264+
* @param ZipStream $zip
265+
*/
266+
private function compressPhotosFromPaginator(LengthAwarePaginator $paginator, AbstractAlbum $album, string $full_name_of_directory, array &$used_file_names, ZipStream $zip): void
267+
{
268+
$current_page = 1;
269+
$last_page = $paginator->lastPage();
270+
271+
// Process first page (already loaded)
272+
$this->compressPhotosFromCollection($paginator->getCollection(), $album, $full_name_of_directory, $used_file_names, $zip);
273+
274+
// Process remaining pages
275+
while ($current_page < $last_page) {
276+
$current_page++;
277+
/** @var LengthAwarePaginator<int,Photo> $next_page */
278+
$next_page = $album->photos()->paginate($paginator->perPage(), ['*'], 'page', $current_page);
279+
$this->compressPhotosFromCollection($next_page->getCollection(), $album, $full_name_of_directory, $used_file_names, $zip);
280+
}
281+
}
282+
283+
/**
284+
* Compresses photos from a collection.
285+
*
286+
* @param Collection<int,Photo>|iterable<Photo> $photos
287+
* @param AbstractAlbum $album
288+
* @param string $full_name_of_directory
289+
* @param array<string> $used_file_names
290+
* @param ZipStream $zip
291+
*/
292+
private function compressPhotosFromCollection(Collection|iterable $photos, AbstractAlbum $album, string $full_name_of_directory, array &$used_file_names, ZipStream $zip): void
293+
{
232294
/** @var Photo $photo */
233295
foreach ($photos as $photo) {
234296
try {
@@ -265,22 +327,6 @@ private function compressAlbum(AbstractAlbum $album, array &$used_dir_names, ?st
265327
}
266328
// @codeCoverageIgnoreEnd
267329
}
268-
269-
// Recursively compress sub-albums
270-
if ($album instanceof Album) {
271-
$sub_dirs = [];
272-
// TODO: For higher efficiency, ensure that the photos of each child album together with the original size variant are eagerly loaded.
273-
$sub_albums = $album->children;
274-
foreach ($sub_albums as $sub_album) {
275-
try {
276-
$this->compressAlbum($sub_album, $sub_dirs, $full_name_of_directory, $zip);
277-
// @codeCoverageIgnoreStart
278-
} catch (\Throwable $e) {
279-
Handler::reportSafely($e);
280-
}
281-
// @codeCoverageIgnoreEnd
282-
}
283-
}
284330
}
285331

286332
/**

app/Http/Requests/Album/ZipRequest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
use App\Policies\AlbumPolicy;
2525
use App\Policies\PhotoPolicy;
2626
use App\Rules\AlbumIDListRule;
27+
use App\Rules\AlbumIDRule;
2728
use App\Rules\RandomIDListRule;
28-
use App\Rules\RandomIDRule;
2929
use Illuminate\Support\Facades\Gate;
3030
use Illuminate\Validation\Rules\Enum;
3131

@@ -71,7 +71,7 @@ public function rules(): array
7171
RequestAttribute::ALBUM_IDS_ATTRIBUTE => ['sometimes', new AlbumIDListRule()],
7272
RequestAttribute::PHOTO_IDS_ATTRIBUTE => ['sometimes', new RandomIDListRule()],
7373
RequestAttribute::SIZE_VARIANT_ATTRIBUTE => ['required_if_accepted:photos_ids', new Enum(DownloadVariantType::class)],
74-
RequestAttribute::FROM_ID_ATTRIBUTE => ['required_if_accepted:photos_ids', new RandomIDRule(true)],
74+
RequestAttribute::FROM_ID_ATTRIBUTE => ['required_if_accepted:photos_ids', new AlbumIDRule(true)],
7575
];
7676
}
7777

tests/Unit/Http/Requests/Album/ZipRequestTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
use App\Models\TagAlbum;
1818
use App\Policies\AlbumPolicy;
1919
use App\Rules\AlbumIDListRule;
20+
use App\Rules\AlbumIDRule;
2021
use App\Rules\RandomIDListRule;
21-
use App\Rules\RandomIDRule;
2222
use Illuminate\Support\Facades\Gate;
2323
use Illuminate\Validation\Rules\Enum;
2424
use Tests\Unit\Http\Requests\Base\BaseRequestTest;
@@ -63,7 +63,7 @@ public function testRules(): void
6363
RequestAttribute::ALBUM_IDS_ATTRIBUTE => ['sometimes', new AlbumIDListRule()],
6464
RequestAttribute::PHOTO_IDS_ATTRIBUTE => ['sometimes', new RandomIDListRule()],
6565
RequestAttribute::SIZE_VARIANT_ATTRIBUTE => ['required_if_accepted:photos_ids', new Enum(DownloadVariantType::class)],
66-
RequestAttribute::FROM_ID_ATTRIBUTE => ['required_if_accepted:photos_ids', new RandomIDRule(true)],
66+
RequestAttribute::FROM_ID_ATTRIBUTE => ['required_if_accepted:photos_ids', new AlbumIDRule(true)],
6767
];
6868
$this->assertCount(count($expectedRuleMap), $rules); // only validating the first 7 rules & the GRANTS_UPLOAD_ATTRIBUTE is tested afterwards
6969

0 commit comments

Comments
 (0)