Skip to content

Commit

Permalink
[Stacked 4] Refactor deletion (#2382)
Browse files Browse the repository at this point in the history
* [Stacked 5] Remove local requirement Middleware (#2383)
* [Stacked 6] Remove hard coded values from naming strategy (#2384)
* [Stacked 7] Finally upload the data to S3 (#2385)
  • Loading branch information
ildyria authored Apr 24, 2024
1 parent 9b1a7be commit 56458de
Show file tree
Hide file tree
Showing 16 changed files with 294 additions and 148 deletions.
3 changes: 2 additions & 1 deletion app/Actions/Album/Delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use App\Constants\AccessPermissionConstants as APC;
use App\Contracts\Exceptions\InternalLycheeException;
use App\Enum\SmartAlbumType;
use App\Enum\StorageDiskType;
use App\Exceptions\Internal\LycheeAssertionError;
use App\Exceptions\Internal\QueryBuilderException;
use App\Exceptions\ModelDBException;
Expand Down Expand Up @@ -107,7 +108,7 @@ public function do(array $albumIDs): FileDeleter
// Delete the photos from DB and obtain the list of files which need
// to be deleted later
$fileDeleter = (new PhotoDelete())->do($unsortedPhotoIDs, $recursiveAlbumIDs);
$fileDeleter->addRegularFiles($recursiveAlbumTracks);
$fileDeleter->addFiles($recursiveAlbumTracks, StorageDiskType::LOCAL->value);

// Remove the sub-forest spanned by the regular albums
$this->deleteSubForest($albums);
Expand Down
5 changes: 3 additions & 2 deletions app/Actions/Photo/Archive.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use App\Actions\Photo\Extensions\ArchiveFileInfo;
use App\Contracts\Exceptions\LycheeException;
use App\Contracts\Models\AbstractSizeVariantNamingStrategy;
use App\Enum\DownloadVariantType;
use App\Enum\SizeVariantType;
use App\Exceptions\ConfigurationKeyMissingException;
Expand All @@ -14,6 +13,7 @@
use App\Models\Configs;
use App\Models\Photo;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Support\Facades\Storage;
use Safe\Exceptions\InfoException;
use function Safe\fclose;
use function Safe\fopen;
Expand Down Expand Up @@ -284,7 +284,8 @@ protected function extractFileInfo(Photo $photo, DownloadVariantType $downloadVa
$baseFilename = $validFilename !== '' ? $validFilename : 'Untitled';

if ($downloadVariant === DownloadVariantType::LIVEPHOTOVIDEO) {
$sourceFile = new FlysystemFile(AbstractSizeVariantNamingStrategy::getImageDisk(), $photo->live_photo_short_path);
$disk = $photo->size_variants->getSizeVariant(SizeVariantType::ORIGINAL)->storage_disk->value;
$sourceFile = new FlysystemFile(Storage::disk($disk), $photo->live_photo_short_path);
$baseFilenameAddon = '';
} else {
$sv = $photo->size_variants->getSizeVariant($downloadVariant->getSizeVariantType());
Expand Down
2 changes: 2 additions & 0 deletions app/Actions/Photo/Create.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ private function handleStandalone(InitDTO $initDTO): Photo
Shared\Save::class,
Standalone\CreateOriginalSizeVariant::class,
Standalone\CreateSizeVariants::class,
Shared\UploadSizeVariantsToS3::class,
];

return $this->executePipeOnDTO($pipes, $dto)->getPhoto();
Expand Down Expand Up @@ -244,6 +245,7 @@ private function handlePhotoLivePartner(InitDTO $initDTO): Photo
Shared\Save::class,
Standalone\CreateOriginalSizeVariant::class,
Standalone\CreateSizeVariants::class,
Shared\UploadSizeVariantsToS3::class,
];
$standAloneDto = $this->executePipeOnDTO($standAlonePipes, $standAloneDto);

Expand Down
49 changes: 35 additions & 14 deletions app/Actions/Photo/Delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace App\Actions\Photo;

use App\Enum\SizeVariantType;
use App\Exceptions\Internal\LycheeAssertionError;
use App\Exceptions\Internal\QueryBuilderException;
use App\Exceptions\ModelDBException;
Expand Down Expand Up @@ -108,9 +109,10 @@ private function collectSizeVariantPathsByPhotoID(array $photoIDs): void
return;
}

$svShortPaths = SizeVariant::query()
// Maybe consider doing multiple queries for the different storage types.
$sizeVariants = SizeVariant::query()
->from('size_variants as sv')
->select(['sv.short_path'])
->select(['sv.short_path', 'sv.storage_disk'])
->join('photos as p', 'p.id', '=', 'sv.photo_id')
->leftJoin('photos as dup', function (JoinClause $join) use ($photoIDs) {
$join
Expand All @@ -119,8 +121,8 @@ private function collectSizeVariantPathsByPhotoID(array $photoIDs): void
})
->whereIn('p.id', $photoIDs)
->whereNull('dup.id')
->pluck('sv.short_path');
$this->fileDeleter->addRegularFilesOrSymbolicLinks($svShortPaths);
->get();
$this->fileDeleter->addSizeVariants($sizeVariants);
// @codeCoverageIgnoreStart
} catch (\InvalidArgumentException $e) {
throw LycheeAssertionError::createFromUnexpectedException($e);
Expand Down Expand Up @@ -148,9 +150,10 @@ private function collectSizeVariantPathsByAlbumID(array $albumIDs): void
return;
}

$svShortPaths = SizeVariant::query()
// Maybe consider doing multiple queries for the different storage types.
$sizeVariants = SizeVariant::query()
->from('size_variants as sv')
->select(['sv.short_path'])
->select(['sv.short_path', 'sv.storage_disk'])
->join('photos as p', 'p.id', '=', 'sv.photo_id')
->leftJoin('photos as dup', function (JoinClause $join) use ($albumIDs) {
$join
Expand All @@ -159,8 +162,8 @@ private function collectSizeVariantPathsByAlbumID(array $albumIDs): void
})
->whereIn('p.album_id', $albumIDs)
->whereNull('dup.id')
->pluck('sv.short_path');
$this->fileDeleter->addRegularFilesOrSymbolicLinks($svShortPaths);
->get();
$this->fileDeleter->addSizeVariants($sizeVariants);
// @codeCoverageIgnoreStart
} catch (\InvalidArgumentException $e) {
throw LycheeAssertionError::createFromUnexpectedException($e);
Expand Down Expand Up @@ -190,7 +193,12 @@ private function collectLivePhotoPathsByPhotoID(array $photoIDs)

$livePhotoShortPaths = Photo::query()
->from('photos as p')
->select(['p.live_photo_short_path'])
->select(['p.live_photo_short_path', 'sv.storage_disk'])
->join('size_variants as sv', function (JoinClause $join) {
$join
->on('sv.photo_id', '=', 'p.id')
->where('sv.type', '=', SizeVariantType::ORIGINAL);
})
->leftJoin('photos as dup', function (JoinClause $join) use ($photoIDs) {
$join
->on('dup.live_photo_checksum', '=', 'p.live_photo_checksum')
Expand All @@ -199,8 +207,12 @@ private function collectLivePhotoPathsByPhotoID(array $photoIDs)
->whereIn('p.id', $photoIDs)
->whereNull('dup.id')
->whereNotNull('p.live_photo_short_path')
->pluck('p.live_photo_short_path');
$this->fileDeleter->addRegularFilesOrSymbolicLinks($livePhotoShortPaths);
->get(['p.live_photo_short_path', 'sv.storage_disk']);

$liveVariantsShortPathsGrouped = $livePhotoShortPaths->groupBy('storage_disk');
$liveVariantsShortPathsGrouped->each(
fn ($liveVariantsShortPaths, $disk) => $this->fileDeleter->addFiles($liveVariantsShortPaths->map(fn ($lv) => $lv->live_photo_short_path), $disk)
);
// @codeCoverageIgnoreStart
} catch (\InvalidArgumentException $e) {
throw LycheeAssertionError::createFromUnexpectedException($e);
Expand Down Expand Up @@ -230,7 +242,12 @@ private function collectLivePhotoPathsByAlbumID(array $albumIDs)

$livePhotoShortPaths = Photo::query()
->from('photos as p')
->select(['p.live_photo_short_path'])
->select(['p.live_photo_short_path', 'sv.storage_disk'])
->join('size_variants as sv', function (JoinClause $join) {
$join
->on('sv.photo_id', '=', 'p.id')
->where('sv.type', '=', SizeVariantType::ORIGINAL);
})
->leftJoin('photos as dup', function (JoinClause $join) use ($albumIDs) {
$join
->on('dup.live_photo_checksum', '=', 'p.live_photo_checksum')
Expand All @@ -239,8 +256,12 @@ private function collectLivePhotoPathsByAlbumID(array $albumIDs)
->whereIn('p.album_id', $albumIDs)
->whereNull('dup.id')
->whereNotNull('p.live_photo_short_path')
->pluck('p.live_photo_short_path');
$this->fileDeleter->addRegularFilesOrSymbolicLinks($livePhotoShortPaths);
->get(['p.live_photo_short_path', 'sv.storage_disk']);

$liveVariantsShortPathsGrouped = $livePhotoShortPaths->groupBy('storage_disk');
$liveVariantsShortPathsGrouped->each(
fn ($liveVariantsShortPaths, $disk) => $this->fileDeleter->addFiles($liveVariantsShortPaths->map(fn ($lv) => $lv->live_photo_short_path), $disk)
);
// @codeCoverageIgnoreStart
} catch (\InvalidArgumentException $e) {
throw LycheeAssertionError::createFromUnexpectedException($e);
Expand Down
33 changes: 33 additions & 0 deletions app/Actions/Photo/Pipes/Shared/UploadSizeVariantsToS3.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

namespace App\Actions\Photo\Pipes\Shared;

use App\Assets\Features;
use App\Contracts\PhotoCreate\PhotoDTO;
use App\Contracts\PhotoCreate\PhotoPipe;
use App\Jobs\UploadSizeVariantToS3Job;
use App\Models\Configs;
use App\Models\SizeVariant;

/**
* Upload SizeVariants to S3 once done (if enabled).
* Note that we first create the job, then we dispatch it.
* This allows to manage the queue properly and see it in the feedback.
*/
class UploadSizeVariantsToS3 implements PhotoPipe
{
public function handle(PhotoDTO $state, \Closure $next): PhotoDTO
{
if (Features::active('use-s3')) {
$use_job_queues = Configs::getValueAsBool('use_job_queues');

$jobs = $state->getPhoto()->size_variants->toCollection()
->filter(fn ($v) => $v !== null)
->map(fn (SizeVariant $variant) => new UploadSizeVariantToS3Job($variant));

$jobs->each(fn ($job) => $use_job_queues ? dispatch($job) : dispatch_sync($job));
}

return $next($state);
}
}
10 changes: 8 additions & 2 deletions app/Actions/Photo/Pipes/VideoPartner/PlaceVideo.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@
namespace App\Actions\Photo\Pipes\VideoPartner;

use App\Actions\Diagnostics\Pipes\Checks\BasicPermissionCheck;
use App\Contracts\Models\AbstractSizeVariantNamingStrategy;
use App\Assets\Features;
use App\Contracts\PhotoCreate\VideoPartnerPipe;
use App\DTO\PhotoCreate\VideoPartnerDTO;
use App\Enum\StorageDiskType;
use App\Exceptions\ConfigurationException;
use App\Exceptions\Handler;
use App\Exceptions\Internal\LycheeAssertionError;
use App\Exceptions\MediaFileOperationException;
use App\Image\Files\FlysystemFile;
use App\Image\Files\NativeLocalFile;
use App\Image\StreamStat;
use Illuminate\Support\Facades\Storage;

/**
* Puts the video source file into the final position at video target file.
Expand Down Expand Up @@ -47,7 +49,11 @@ class PlaceVideo implements VideoPartnerPipe
{
public function handle(VideoPartnerDTO $state, \Closure $next): VideoPartnerDTO
{
$videoTargetFile = new FlysystemFile(AbstractSizeVariantNamingStrategy::getImageDisk(), $state->videoPath);
$disk = Storage::disk(StorageDiskType::LOCAL->value);
if (Features::active('use-s3')) {
$disk = Storage::disk(StorageDiskType::S3->value);
}
$videoTargetFile = new FlysystemFile($disk, $state->videoPath);

try {
if ($state->videoFile instanceof NativeLocalFile) {
Expand Down
8 changes: 4 additions & 4 deletions app/Actions/SizeVariant/Delete.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,18 @@ public function do(array $svIDs): FileDeleter
// Get all short paths of size variants which are going to be deleted.
// But exclude those short paths which are duplicated by a size
// variant which is not going to be deleted.
$svShortPaths = SizeVariant::query()
$sizeVariants = SizeVariant::query()
->from('size_variants as sv')
->select(['sv.short_path'])
->select(['sv.short_path', 'sv.storage_disk'])
->leftJoin('size_variants as dup', function (JoinClause $join) use ($svIDs) {
$join
->on('dup.short_path', '=', 'sv.short_path')
->whereNotIn('dup.id', $svIDs);
})
->whereIn('sv.id', $svIDs)
->whereNull('dup.id')
->pluck('sv.short_path');
$fileDeleter->addRegularFilesOrSymbolicLinks($svShortPaths);
->get();
$fileDeleter->addSizeVariants($sizeVariants);

// Get all short paths of symbolic links which point to size variants
// which are going to be deleted
Expand Down
11 changes: 9 additions & 2 deletions app/Console/Commands/Ghostbuster.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

namespace App\Console\Commands;

use App\Assets\Features;
use App\Console\Commands\Utilities\Colorize;
use App\Contracts\Models\AbstractSizeVariantNamingStrategy;
use App\Enum\SizeVariantType;
use App\Enum\StorageDiskType;
use App\Exceptions\UnexpectedException;
use App\Models\Photo;
use App\Models\SizeVariant;
Expand Down Expand Up @@ -73,11 +74,17 @@ public function handle(): int
$removeDeadSymLinks = filter_var($this->option('removeDeadSymLinks'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) === true;
$removeZombiePhotos = filter_var($this->option('removeZombiePhotos'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) === true;
$dryrun = filter_var($this->option('dryrun'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) !== false;
$uploadDisk = AbstractSizeVariantNamingStrategy::getImageDisk();
$uploadDisk = Features::active('use-s3')
? Storage::disk(StorageDiskType::S3->value)
: Storage::disk(StorageDiskType::LOCAL->value);
$symlinkDisk = Storage::disk(SymLink::DISK_NAME);
$isLocalDisk = $uploadDisk->getAdapter() instanceof LocalFilesystemAdapter;

$this->line('');
if (!$isLocalDisk) {
$this->line($this->col->red('Using non-local disk to store images, USE AT YOUR OWN RISKS! This code is not battle tested.'));
$this->line('');
}

if ($removeDeadSymLinks && !$isLocalDisk) {
$this->line($this->col->yellow('Removal of dead symlinks requested, but filesystem does not support symlinks.'));
Expand Down
17 changes: 0 additions & 17 deletions app/Contracts/Models/AbstractSizeVariantNamingStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,15 @@
use App\Enum\SizeVariantType;
use App\Image\Files\FlysystemFile;
use App\Models\Photo;
use Illuminate\Filesystem\FilesystemAdapter;
use Illuminate\Support\Facades\Storage;

/**
* Interface SizeVariantNamingStrategy.
*/
abstract class AbstractSizeVariantNamingStrategy
{
/**
* The name of the Flysystem disk where images are stored.
*/
public const IMAGE_DISK_NAME = 'images';

protected string $extension = '';
protected ?Photo $photo = null;

/**
* Returns the disk on which the size variants are put.
*
* @return FilesystemAdapter
*/
public static function getImageDisk(): FilesystemAdapter
{
return Storage::disk(self::IMAGE_DISK_NAME);
}

/**
* Sets the extension to be used for the size variants.
*
Expand Down
11 changes: 11 additions & 0 deletions app/Exceptions/Internal/FileDeletionException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace App\Exceptions\Internal;

class FileDeletionException extends LycheeDomainException
{
public function __construct(string $storage, string $path)
{
parent::__construct(sprintf('Storage::delete (%s) failed: %s', $storage, $path));
}
}
1 change: 0 additions & 1 deletion app/Http/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ class Kernel extends HttpKernel
'installation' => \App\Http\Middleware\InstallationStatus::class,
'admin_user' => \App\Http\Middleware\AdminUserStatus::class,
'migration' => \App\Http\Middleware\MigrationStatus::class,
'local_storage' => \App\Http\Middleware\LocalStorageOnly::class,
'content_type' => \App\Http\Middleware\ContentType::class,
'accept_content_type' => \App\Http\Middleware\AcceptContentType::class,
'redirect-legacy-id' => \App\Http\Middleware\RedirectLegacyPhotoID::class,
Expand Down
31 changes: 0 additions & 31 deletions app/Http/Middleware/LocalStorageOnly.php

This file was deleted.

Loading

0 comments on commit 56458de

Please sign in to comment.