Consistent JSON API incl. Proper Handling of Errors (2nd half of big refactoring)

.env.example

@@ -67,3 +67,10 @@ PUSHER_APP_CLUSTER=mt1
 
 MIX_PUSHER_APP_KEY="${PUSHER_APP_KEY}"
 MIX_PUSHER_APP_CLUSTER="${PUSHER_APP_CLUSTER}"
+
+# The trusted proxies if Lychee is behind a reverse proxy
+# Accepted values:
+#  - `null`: no proxy
+#  - `*`: any proxy
+#  - <ip address>[,<ip address>]: a comma-seperated list of IP addresses
+TRUSTED_PROXIES=null

.env.homestead

@@ -54,3 +54,10 @@ PUSHER_APP_CLUSTER=mt1
 
 MIX_PUSHER_APP_KEY="${PUSHER_APP_KEY}"
 MIX_PUSHER_APP_CLUSTER="${PUSHER_APP_CLUSTER}"
+
+# The trusted proxies if Lychee is behind a reverse proxy
+# Accepted values:
+#  - `null`: no proxy
+#  - `*`: any proxy
+#  - <ip address>[,<ip address>]: a comma-seperated list of IP addresses
+TRUSTED_PROXIES=null

app/Actions/Album/Archive.php

@@ -3,15 +3,16 @@
 namespace App\Actions\Album;
 
 use App\Contracts\AbstractAlbum;
-use App\Contracts\BaseAlbum;
+use App\Exceptions\Internal\FrameworkException;
 use App\Facades\AccessControl;
 use App\Facades\Helpers;
 use App\Models\Album;
 use App\Models\Configs;
-use App\Models\Logs;
+use App\Models\Extensions\BaseAlbum;
 use App\Models\Photo;
 use App\Models\TagAlbum;
 use App\SmartAlbums\BaseSmartAlbum;
+use Illuminate\Contracts\Debug\ExceptionHandler;
 use Illuminate\Support\Collection;
 use Symfony\Component\HttpFoundation\HeaderUtils;
 use Symfony\Component\HttpFoundation\StreamedResponse;
@@ -29,16 +30,24 @@ class Archive extends Action
 		'<', '>', ':', '"', '/', '\\', '|', '?', '*',
 	];
 
+	protected ExceptionHandler $exceptionHandler;
+
+	public function __construct()
+	{
+		parent::__construct();
+		$this->exceptionHandler = resolve(ExceptionHandler::class);
+	}
+
 	/**
-	 * @param array $albumIDs
+	 * @param Collection<AbstractAlbum> $albums
 	 *
 	 * @return StreamedResponse
+	 *
+	 * @throws FrameworkException
 	 */
-	public function do(array $albumIDs): StreamedResponse
+	public function do(Collection $albums): StreamedResponse
 	{
-		$albums = $this->albumFactory->findWhereIDsIn($albumIDs);
-
-		$response = new StreamedResponse(function () use ($albums) {
+		$responseGenerator = function () use ($albums) {
 			$options = new \ZipStream\Option\Archive();
 			$options->setEnableZip64(Configs::get_value('zip64', '1') === '1');
 			$zip = new ZipStream(null, $options);
@@ -50,22 +59,27 @@ public function do(array $albumIDs): StreamedResponse
 
 			// finish the zip stream
 			$zip->finish();
-		});
-
-		// Set file type and destination
-		$zipTitle = self::createZipTitle($albums);
-		$disposition = HeaderUtils::makeDisposition(
-			HeaderUtils::DISPOSITION_ATTACHMENT,
-			$zipTitle . '.zip',
-			mb_check_encoding($zipTitle, 'ASCII') ? '' : 'Album.zip'
-		);
-		$response->headers->set('Content-Type', 'application/x-zip');
-		$response->headers->set('Content-Disposition', $disposition);
-
-		// Disable caching
-		$response->headers->set('Cache-Control', 'no-cache, no-store, must-revalidate');
-		$response->headers->set('Pragma', 'no-cache');
-		$response->headers->set('Expires', '0');
+		};
+
+		try {
+			$response = new StreamedResponse($responseGenerator);
+			// Set file type and destination
+			$zipTitle = self::createZipTitle($albums);
+			$disposition = HeaderUtils::makeDisposition(
+				HeaderUtils::DISPOSITION_ATTACHMENT,
+				$zipTitle . '.zip',
+				mb_check_encoding($zipTitle, 'ASCII') ? '' : 'Album.zip'
+			);
+			$response->headers->set('Content-Type', 'application/x-zip');
+			$response->headers->set('Content-Disposition', $disposition);
+
+			// Disable caching
+			$response->headers->set('Cache-Control', 'no-cache, no-store, must-revalidate');
+			$response->headers->set('Pragma', 'no-cache');
+			$response->headers->set('Expires', '0');
+		} catch (\InvalidArgumentException $e) {
+			throw new FrameworkException('Symfony\'s response component', $e);
+		}
 
 		return $response;
 	}
@@ -154,43 +168,48 @@ private function compressAlbum(AbstractAlbum $album, array &$usedDirNames, ?stri
 		}
 
 		$usedFileNames = [];
+		// 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`.
 		$photos = $album->photos;
 
 		/** @var Photo $photo */
 		foreach ($photos as $photo) {
-			// For photos in smart or tag albums, skip the ones that are not
-			// downloadable based on their actual parent album.  The test for
-			// album_id == null shouldn't really be needed as all such photos
-			// in smart albums should be owned by the current user...
-			if (($album instanceof BaseSmartAlbum || $album instanceof TagAlbum) &&
-			!AccessControl::is_current_user($photo->owner_id) &&
-			!($photo->album_id == null ? $album->is_downloadable : $photo->album->is_downloadable)) {
-				continue;
-			}
-
-			$fullPath = $photo->size_variants->getOriginal()->full_path;
-			// Check if readable
-			if (!@is_readable($fullPath)) {
-				Logs::error(__METHOD__, __LINE__, 'Original photo missing: ' . $fullPath);
-				continue;
+			try {
+				// For photos in smart or tag albums, skip the ones that are not
+				// downloadable based on their actual parent album.  The test for
+				// album_id == null shouldn't really be needed as all such photos
+				// in smart albums should be owned by the current user...
+				if (($album instanceof BaseSmartAlbum || $album instanceof TagAlbum) &&
+					!AccessControl::is_current_user_or_admin($photo->owner_id) &&
+					!($photo->album_id == null ? $album->is_downloadable : $photo->album->is_downloadable)) {
+					continue;
+				}
+
+				$fullPath = $photo->size_variants->getOriginal()->full_path;
+
+				// Set title for photo
+				$extension = Helpers::getExtension($fullPath, false);
+				$fileBaseName = $this->makeUnique(self::createValidTitle($photo->title), $usedFileNames);
+				$fileName = $fullNameOfDirectory . '/' . $fileBaseName . $extension;
+
+				// Reset the execution timeout for every iteration.
+				set_time_limit(ini_get('max_execution_time'));
+				$zip->addFileFromPath($fileName, $fullPath);
+			} catch (\Throwable $e) {
+				$this->exceptionHandler->report($e);
 			}
-
-			// Set title for photo
-			$extension = Helpers::getExtension($fullPath, false);
-			$fileBaseName = $this->makeUnique(self::createValidTitle($photo->title), $usedFileNames);
-			$fileName = $fullNameOfDirectory . '/' . $fileBaseName . $extension;
-
-			// Reset the execution timeout for every iteration.
-			set_time_limit(ini_get('max_execution_time'));
-			$zip->addFileFromPath($fileName, $fullPath);
 		}
 
 		// Recursively compress sub-albums
 		if ($album instanceof Album) {
 			$subDirs = [];
+			// TODO: For higher efficiency, ensure that the photos of each child album together with the original size variant are eagerly loaded.
 			$subAlbums = $album->children;
 			foreach ($subAlbums as $subAlbum) {
-				$this->compressAlbum($subAlbum, $subDirs, $fullNameOfDirectory, $zip);
+				try {
+					$this->compressAlbum($subAlbum, $subDirs, $fullNameOfDirectory, $zip);
+				} catch (\Throwable $e) {
+					$this->exceptionHandler->report($e);
+				}
 			}
 		}
 	}
@@ -207,6 +226,6 @@ private static function isArchivable(AbstractAlbum $album): bool
 		return
 			$album->is_downloadable ||
 			($album instanceof BaseSmartAlbum && AccessControl::is_logged_in()) ||
-			($album instanceof BaseAlbum && AccessControl::is_current_user($album->owner_id));
+			($album instanceof BaseAlbum && AccessControl::is_current_user_or_admin($album->owner_id));
 	}
 }

app/Actions/Album/Create.php

@@ -2,46 +2,45 @@
 
 namespace App\Actions\Album;
 
+use App\Exceptions\ModelDBException;
 use App\Facades\AccessControl;
 use App\Models\Album;
 
 class Create extends Action
 {
 	/**
-	 * @param string      $title
-	 * @param string|null $parent_id
+	 * @param string     $title
+	 * @param Album|null $parentAlbum
 	 *
 	 * @return Album
+	 *
+	 * @throws ModelDBException
 	 */
-	public function create(string $title, ?string $parent_id = null): Album
+	public function create(string $title, ?Album $parentAlbum): Album
 	{
 		$album = new Album();
 		$album->title = $title;
-		$this->set_parent($album, $parent_id);
-		if (!$album->save()) {
-			throw new \RuntimeException('could not persist album to DB');
-		}
+		$this->set_parent($album, $parentAlbum);
+		$album->save();
 
 		return $album;
 	}
 
 	/**
 	 * Setups parent album on album structure.
 	 *
-	 * @param Album       $album
-	 * @param string|null $parent_id
+	 * @param Album      $album
+	 * @param Album|null $parentAlbum
 	 */
-	private function set_parent(Album $album, ?string $parent_id): void
+	private function set_parent(Album $album, ?Album $parentAlbum): void
 	{
-		if ($parent_id !== null) {
-			/** @var Album $parent */
-			$parent = Album::query()->findOrFail($parent_id);
-			// Admin can add subalbums to other users' albums.  Make sure that
+		if ($parentAlbum !== null) {
+			// Admin can add sub-albums to other users' albums.  Make sure that
 			// the ownership stays with that user.
-			$album->owner_id = $parent->owner_id;
+			$album->owner_id = $parentAlbum->owner_id;
 			// Don't set attribute `parent_id` manually, but use specialized
 			// methods of the nested set `NodeTrait`.
-			$album->appendToNode($parent);
+			$album->appendToNode($parentAlbum);
 		} else {
 			$album->owner_id = AccessControl::id();
 			$album->makeRoot();

app/Actions/Album/CreateTagAlbum.php

@@ -2,6 +2,7 @@
 
 namespace App\Actions\Album;
 
+use App\Exceptions\ModelDBException;
 use App\Facades\AccessControl;
 use App\Models\TagAlbum;
 
@@ -10,20 +11,20 @@ class CreateTagAlbum extends Action
 	/**
 	 * Create a new smart album based on tags.
 	 *
-	 * @param string $title
-	 * @param string $show_tags
+	 * @param string   $title
+	 * @param string[] $show_tags
 	 *
 	 * @return TagAlbum
+	 *
+	 * @throws ModelDBException
 	 */
-	public function create(string $title, string $show_tags): TagAlbum
+	public function create(string $title, array $show_tags): TagAlbum
 	{
 		$album = new TagAlbum();
 		$album->title = $title;
 		$album->show_tags = $show_tags;
 		$album->owner_id = AccessControl::id();
-		if (!$album->save()) {
-			throw new \RuntimeException('could not persist album to DB');
-		}
+		$album->save();
 
 		return $album;
 	}

app/Actions/Album/Delete.php

@@ -3,6 +3,9 @@
 namespace App\Actions\Album;
 
 use App\Actions\Photo\Delete as PhotoDelete;
+use App\Contracts\InternalLycheeException;
+use App\Exceptions\Internal\QueryBuilderException;
+use App\Exceptions\ModelDBException;
 use App\Facades\AccessControl;
 use App\Image\FileDeleter;
 use App\Models\Album;
@@ -50,6 +53,8 @@ class Delete extends Action
 	 * @param string[] $albumIDs the album IDs
 	 *
 	 * @return FileDeleter contains the collected files which became obsolete
+	 *
+	 * @throws ModelDBException
 	 */
 	public function do(array $albumIDs): FileDeleter
 	{
@@ -117,14 +122,22 @@ public function do(array $albumIDs): FileDeleter
 			})->delete();
 
 			return $fileDeleter;
-		} catch (\Exception $e) {
+		} catch (QueryBuilderException|InternalLycheeException $e) {
+			try {
+				// if anything goes wrong, don't leave the tree in an inconsistent state
+				Album::query()->fixTree();
+			} catch (\Throwable) {
+				// Sic! We cannot do anything about the inner exception
+			}
+			throw ModelDBException::create('albums', 'deleting', $e);
+		} catch (\InvalidArgumentException $e) {
 			try {
 				// if anything goes wrong, don't leave the tree in an inconsistent state
 				Album::query()->fixTree();
 			} catch (\Throwable) {
 				// Sic! We cannot do anything about the inner exception
 			}
-			throw $e;
+			assert(false, new \AssertionError('\InvalidArgumentException must not be thrown by ->where', $e->getCode(), $e));
 		}
 	}
 }