diff --git a/.env.example b/.env.example index 17c5eb55bfb..0ecc09ef3c6 100644 --- a/.env.example +++ b/.env.example @@ -66,6 +66,8 @@ DB_LIST_FOREIGN_KEYS=true CACHE_DRIVER=file SESSION_DRIVER=file SESSION_LIFETIME=120 +# `sync` if jobs needs to be executed live (default) or `database` if they can be defered. +QUEUE_CONNECTION=sync SECURITY_HEADER_HSTS_ENABLE=false SESSION_SECURE_COOKIE=false diff --git a/.github/workflows/.env.mariadb b/.github/workflows/.env.mariadb index 4cfb6104ec5..ff63b84d058 100644 --- a/.github/workflows/.env.mariadb +++ b/.github/workflows/.env.mariadb @@ -14,4 +14,4 @@ DB_LIST_FOREIGN_KEYS=true CACHE_DRIVER=array SESSION_DRIVER=array -QUEUE_DRIVER=sync +QUEUE_CONNECTION=sync diff --git a/.github/workflows/.env.postgresql b/.github/workflows/.env.postgresql index f81b051b5be..ce31ff92ff1 100644 --- a/.github/workflows/.env.postgresql +++ b/.github/workflows/.env.postgresql @@ -13,4 +13,4 @@ DB_PASSWORD=postgres CACHE_DRIVER=array SESSION_DRIVER=array -QUEUE_DRIVER=sync +QUEUE_CONNECTION=sync diff --git a/.github/workflows/.env.sqlite b/.github/workflows/.env.sqlite index f9867edd8bd..ff3c90395bc 100644 --- a/.github/workflows/.env.sqlite +++ b/.github/workflows/.env.sqlite @@ -9,4 +9,4 @@ DB_LIST_FOREIGN_KEYS=true CACHE_DRIVER=array SESSION_DRIVER=array -QUEUE_DRIVER=sync +QUEUE_CONNECTION=sync diff --git a/app/Http/Controllers/PhotoController.php b/app/Http/Controllers/PhotoController.php index d4eae2ba579..967a2c6a69e 100644 --- a/app/Http/Controllers/PhotoController.php +++ b/app/Http/Controllers/PhotoController.php @@ -3,16 +3,14 @@ namespace App\Http\Controllers; use App\Actions\Photo\Archive; -use App\Actions\Photo\Create; use App\Actions\Photo\Delete; use App\Actions\Photo\Duplicate; -use App\Actions\Photo\Strategies\ImportMode; use App\Actions\User\Notify; use App\Contracts\Exceptions\InternalLycheeException; use App\Contracts\Exceptions\LycheeException; use App\Exceptions\MediaFileOperationException; use App\Exceptions\ModelDBException; -use App\Exceptions\UnauthenticatedException; +use App\Factories\AlbumFactory; use App\Http\Requests\Photo\AddPhotoRequest; use App\Http\Requests\Photo\ArchivePhotosRequest; use App\Http\Requests\Photo\ClearSymLinkRequest; @@ -28,8 +26,9 @@ use App\Http\Requests\Photo\SetPhotosTitleRequest; use App\Http\Requests\Photo\SetPhotoUploadDateRequest; use App\Http\Resources\Models\PhotoResource; -use App\Image\Files\TemporaryLocalFile; +use App\Image\Files\ProcessableJobFile; use App\Image\Files\UploadedFile; +use App\Jobs\ProcessImageJob; use App\ModelFunctions\SymLinkFunctions; use App\Models\Configs; use App\Models\Photo; @@ -38,20 +37,18 @@ use Illuminate\Http\JsonResponse; use Illuminate\Routing\Controller; use Illuminate\Support\Facades\App; -use Illuminate\Support\Facades\Auth; use Symfony\Component\HttpFoundation\Response as SymfonyResponse; class PhotoController extends Controller { - private SymLinkFunctions $symLinkFunctions; - /** * @param SymLinkFunctions $symLinkFunctions + * @param AlbumFactory $albumFactory */ public function __construct( - SymLinkFunctions $symLinkFunctions + private SymLinkFunctions $symLinkFunctions, + private AlbumFactory $albumFactory ) { - $this->symLinkFunctions = $symLinkFunctions; } /** @@ -93,16 +90,13 @@ public function getRandom(): PhotoResource * * @param AddPhotoRequest $request * - * @return PhotoResource + * @return PhotoResource|JsonResponse * * @throws LycheeException * @throws ModelNotFoundException */ - public function add(AddPhotoRequest $request): PhotoResource + public function add(AddPhotoRequest $request): PhotoResource|JsonResponse { - /** @var int $currentUserId */ - $currentUserId = Auth::id() ?? throw new UnauthenticatedException(); - // This code is a nasty work-around which should not exist. // PHP stores a temporary copy of the uploaded file without a file // extension. @@ -121,23 +115,25 @@ public function add(AddPhotoRequest $request): PhotoResource // Hence, we must make a deep copy. // TODO: Remove this code again, if all other TODOs regarding MIME and file handling are properly refactored and we have stopped using absolute file paths as the least common denominator to pass around files. $uploadedFile = new UploadedFile($request->uploadedFile()); - $copiedFile = new TemporaryLocalFile( + $processableFile = new ProcessableJobFile( $uploadedFile->getOriginalExtension(), $uploadedFile->getOriginalBasename() ); - $copiedFile->write($uploadedFile->read()); + $processableFile->write($uploadedFile->read()); + $uploadedFile->close(); $uploadedFile->delete(); + $processableFile->close(); // End of work-around - // As the file has been uploaded, the (temporary) source file shall be - // deleted - $create = new Create( - new ImportMode(deleteImported: true, skipDuplicates: Configs::getValueAsBool('skip_duplicates')), - $currentUserId - ); + if (Configs::getValueAsBool('use_job_queues')) { + ProcessImageJob::dispatch($processableFile, $request->album()); + + return new JsonResponse(null, 201); + } - $photo = $create->add($copiedFile, $request->album()); + $job = new ProcessImageJob($processableFile, $request->album()); + $photo = $job->handle($this->albumFactory); $isNew = $photo->created_at->toIso8601String() === $photo->updated_at->toIso8601String(); return PhotoResource::make($photo)->setStatus($isNew ? 201 : 200); diff --git a/app/Image/Files/LoadTemporaryFileTrait.php b/app/Image/Files/LoadTemporaryFileTrait.php new file mode 100644 index 00000000000..c3d64270a62 --- /dev/null +++ b/app/Image/Files/LoadTemporaryFileTrait.php @@ -0,0 +1,54 @@ +getFileBasePath() . + DIRECTORY_SEPARATOR . + strtr(base64_encode(random_bytes(12)), '+/', '-_') . + $fileExtension; + $this->stream = fopen($tempFilePath, 'x+b'); + } catch (\ErrorException|\Exception $e) { + $tempFilePath = null; + $lastException = $e; + } + } while ($tempFilePath === null && $retryCounter > 0); + if ($tempFilePath === null) { + throw new MediaFileOperationException('unable to create temporary file', $lastException); + } + + return $tempFilePath; + } +} diff --git a/app/Image/Files/ProcessableJobFile.php b/app/Image/Files/ProcessableJobFile.php new file mode 100644 index 00000000000..b083400bdea --- /dev/null +++ b/app/Image/Files/ProcessableJobFile.php @@ -0,0 +1,58 @@ +load($fileExtension); + parent::__construct($tempFilePath); + $this->fakeBaseName = $fakeBaseName; + } + + /** + * {@inheritDoc} + */ + protected function getFileBasePath(): string + { + $tempDirPath = storage_path() . DIRECTORY_SEPARATOR . 'image-jobs'; + + if (!file_exists($tempDirPath)) { + mkdir($tempDirPath); + } + + return $tempDirPath; + } + + /** + * {@inheritDoc} + */ + public function getOriginalBasename(): string + { + return $this->fakeBaseName; + } +} diff --git a/app/Image/Files/TemporaryJobFile.php b/app/Image/Files/TemporaryJobFile.php new file mode 100644 index 00000000000..fc651197895 --- /dev/null +++ b/app/Image/Files/TemporaryJobFile.php @@ -0,0 +1,66 @@ +delete(); + parent::__destruct(); + } + + /** + * Load a temporary file with a previously generated file name. + * + * @param string $filePath the path of a Processable Job file + * @param string $fakeBaseName the fake base name of the file; e.g. the original name prior to up-/download + * + * @throws MediaFileOperationException + */ + public function __construct(string $filePath, string $fakeBaseName = '') + { + $lastException = null; + $retryCounter = 5; + do { + try { + $tempFilePath = $filePath; + $retryCounter--; + // We open wih c+b because the file already exists (from ProcessableJobFile) + $this->stream = fopen($tempFilePath, 'c+b'); + } catch (\ErrorException|\Exception $e) { + $tempFilePath = null; + $lastException = $e; + } + } while ($tempFilePath === null && $retryCounter > 0); + if ($tempFilePath === null) { + throw new MediaFileOperationException('unable to create temporary file', $lastException); + } + parent::__construct($tempFilePath); + $this->fakeBaseName = $fakeBaseName; + } + + /** + * {@inheritDoc} + */ + public function getOriginalBasename(): string + { + return $this->fakeBaseName; + } +} diff --git a/app/Image/Files/TemporaryLocalFile.php b/app/Image/Files/TemporaryLocalFile.php index 7acc2ee874d..22a823be14a 100644 --- a/app/Image/Files/TemporaryLocalFile.php +++ b/app/Image/Files/TemporaryLocalFile.php @@ -3,7 +3,6 @@ namespace App\Image\Files; use App\Exceptions\MediaFileOperationException; -use function Safe\fopen; /** * Class TemporaryLocalFile. @@ -13,6 +12,8 @@ */ class TemporaryLocalFile extends NativeLocalFile { + use LoadTemporaryFileTrait; + protected string $fakeBaseName; /** @@ -34,33 +35,19 @@ public function __destruct() */ public function __construct(string $fileExtension, string $fakeBaseName = '') { - // We must not use the usual PHP method `tempnam`, because that - // method does not handle file extensions well, but our temporary - // files need a proper (and correct) extension for the MIME extractor - // to work. - $lastException = null; - $retryCounter = 5; - do { - try { - $tempFilePath = sys_get_temp_dir() . - DIRECTORY_SEPARATOR . - 'lychee-' . - strtr(base64_encode(random_bytes(12)), '+/', '-_') . - $fileExtension; - $retryCounter--; - $this->stream = fopen($tempFilePath, 'x+b'); - } catch (\ErrorException|\Exception $e) { - $tempFilePath = null; - $lastException = $e; - } - } while ($tempFilePath === null && $retryCounter > 0); - if ($tempFilePath === null) { - throw new MediaFileOperationException('unable to create temporary file', $lastException); - } + $tempFilePath = $this->load($fileExtension); parent::__construct($tempFilePath); $this->fakeBaseName = $fakeBaseName; } + /** + * {@inheritDoc} + */ + protected function getFileBasePath(): string + { + return sys_get_temp_dir(); + } + /** * {@inheritDoc} */ diff --git a/app/Jobs/ProcessImageJob.php b/app/Jobs/ProcessImageJob.php new file mode 100644 index 00000000000..6833438e56c --- /dev/null +++ b/app/Jobs/ProcessImageJob.php @@ -0,0 +1,91 @@ +filePath = $file->getPath(); + $this->originalBaseName = $file->getOriginalBasename(); + $this->albumId = $albumId?->id; + $this->userId = Auth::user()->id; + } + + /** + * Execute the job. + * + * Here we handle the execution of the image processing. + * This will create the model, reformat the image etc. + */ + public function handle(AlbumFactory $albumFactory): Photo + { + $copiedFile = new TemporaryJobFile($this->filePath, $this->originalBaseName); + + // As the file has been uploaded, the (temporary) source file shall be + // deleted + $create = new Create( + new ImportMode(deleteImported: true, skipDuplicates: Configs::getValueAsBool('skip_duplicates')), + $this->userId + ); + + $album = null; + if ($this->albumId !== null) { + $album = $albumFactory->findAbstractAlbumOrFail($this->albumId); + } + + return $create->add($copiedFile, $album); + } + + /** + * Catch failures. + * + * @param \Throwable $th + * + * @return void + */ + public function failed(\Throwable $th): void + { + if ($th->getCode() === 999) { + $this->release(); + } else { + logger($th->getMessage()); + Logs::error($th, __LINE__, __FILE__); + } + } +} diff --git a/database/migrations/2023_04_05_150337_create_jobs_table.php b/database/migrations/2023_04_05_150337_create_jobs_table.php new file mode 100644 index 00000000000..869a56c8679 --- /dev/null +++ b/database/migrations/2023_04_05_150337_create_jobs_table.php @@ -0,0 +1,31 @@ +bigIncrements('id'); + $table->string('queue')->index(); + $table->longText('payload'); + $table->unsignedTinyInteger('attempts'); + $table->unsignedInteger('reserved_at')->nullable(); + $table->unsignedInteger('available_at'); + $table->unsignedInteger('created_at'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::dropIfExists('jobs'); + } +}; diff --git a/database/migrations/2023_04_05_150625_queue-processing.php b/database/migrations/2023_04_05_150625_queue-processing.php new file mode 100644 index 00000000000..cf35c2048ed --- /dev/null +++ b/database/migrations/2023_04_05_150625_queue-processing.php @@ -0,0 +1,32 @@ +insert([ + 'key' => 'use_job_queues', + 'value' => '0', + 'cat' => 'Image Processing', + 'type_range' => BOOL, + 'confidentiality' => '0', + 'description' => 'Use job queues instead of directly live connection.', + ]); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + DB::table('configs')->where('key', '=', 'use_job_queues')->delete(); + } +}; diff --git a/database/migrations/2023_04_05_220214_create_failed_jobs_table.php b/database/migrations/2023_04_05_220214_create_failed_jobs_table.php new file mode 100644 index 00000000000..44b8a1e0f95 --- /dev/null +++ b/database/migrations/2023_04_05_220214_create_failed_jobs_table.php @@ -0,0 +1,31 @@ +id(); + $table->string('uuid')->unique(); + $table->text('connection'); + $table->text('queue'); + $table->longText('payload'); + $table->longText('exception'); + $table->timestamp('failed_at')->useCurrent(); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::dropIfExists('failed_jobs'); + } +}; diff --git a/phpunit.xml b/phpunit.xml index 1c949e74f16..f6dd5f3a3a4 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -36,7 +36,7 @@ - + diff --git a/tests/AbstractTestCase.php b/tests/AbstractTestCase.php index df802cb966c..8dc39319a2c 100644 --- a/tests/AbstractTestCase.php +++ b/tests/AbstractTestCase.php @@ -112,6 +112,7 @@ abstract class AbstractTestCase extends BaseTestCase public const CONFIG_PUBLIC_STARRED = 'public_starred'; public const CONFIG_PUBLIC_ON_THIS_DAY = 'public_on_this_day'; public const CONFIG_RAW_FORMATS = 'raw_formats'; + public const CONFIG_USE_JOB_QUEUES = 'use_job_queues'; /** * Visit the given URI with a GET request. diff --git a/tests/Feature/InstallTest.php b/tests/Feature/InstallTest.php index 1cc8c433275..6ca00cafc7f 100644 --- a/tests/Feature/InstallTest.php +++ b/tests/Feature/InstallTest.php @@ -50,6 +50,8 @@ public function testInstall(): void // The order is important: referring tables must be deleted first, referred tables last $tables = [ + 'failed_jobs', + 'jobs', 'sym_links', 'size_variants', 'photos', diff --git a/tests/Feature/PhotosAddMethodsTest.php b/tests/Feature/PhotosAddMethodsTest.php index afd78a3683f..34354a2b896 100644 --- a/tests/Feature/PhotosAddMethodsTest.php +++ b/tests/Feature/PhotosAddMethodsTest.php @@ -13,8 +13,12 @@ namespace Tests\Feature; use App\Image\Files\BaseMediaFile; +use App\Jobs\ProcessImageJob; use App\Models\Configs; +use App\Models\Photo; +use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Queue; use function Safe\copy; use Tests\AbstractTestCase; use Tests\Feature\Base\BasePhotoTest; @@ -244,4 +248,42 @@ public function testRawImportFromUrl(): void Configs::set(self::CONFIG_RAW_FORMATS, $acceptedRawFormats); } } + + public function testJobUploadWithFaking(): void + { + $useJobQueues = Configs::getValueAsString(self::CONFIG_USE_JOB_QUEUES); + try { + Configs::set(self::CONFIG_USE_JOB_QUEUES, '1'); + + Queue::fake(); + Queue::assertNothingPushed(); + + $this->photos_tests->upload( + AbstractTestCase::createUploadedFile(AbstractTestCase::SAMPLE_FILE_NIGHT_IMAGE) + ); + + Queue::assertPushed(ProcessImageJob::class, 1); + self::assertEquals(1, Queue::size()); + } finally { + Configs::set(self::CONFIG_USE_JOB_QUEUES, $useJobQueues); + } + } + + public function testJobUploadWithoutFaking(): void + { + $useJobQueues = Configs::getValueAsString(self::CONFIG_USE_JOB_QUEUES); + $defaultQueue = Config::get('queue.default', 'sync'); + try { + Configs::set(self::CONFIG_USE_JOB_QUEUES, '1'); + Config::set('queue.default', 'sync'); + + $this->photos_tests->upload( + AbstractTestCase::createUploadedFile(AbstractTestCase::SAMPLE_FILE_NIGHT_IMAGE) + ); + self::assertEquals(1, Photo::count()); + } finally { + Configs::set(self::CONFIG_USE_JOB_QUEUES, $useJobQueues); + Config::set('queue.default', $defaultQueue); + } + } }