From 5c318a45b8afd11f6454f4a558a9616786c1a467 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 30 Sep 2023 12:09:29 +0100
Subject: [PATCH] Images: Reverted some thumbnails to be on-demand generated
Added since we can't always be sure of future image usage, and in many
cases we don't generate ahead-of-time.
Also:
- Simplified image handling on certain models.
- Updated various string handling operations to use newer functions.
---
app/Entities/Models/Book.php | 15 +++--------
app/Entities/Models/Bookshelf.php | 20 +++++---------
app/Uploads/Image.php | 5 ++--
app/Uploads/ImageService.php | 45 ++++++++++++++++---------------
app/Users/Models/User.php | 2 +-
5 files changed, 38 insertions(+), 49 deletions(-)
diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php
index fc4556857..f54a0bf2d 100644
--- a/app/Entities/Models/Book.php
+++ b/app/Entities/Models/Book.php
@@ -40,26 +40,19 @@ class Book extends Entity implements HasCoverImage
/**
* Returns book cover image, if book cover not exists return default cover image.
- *
- * @param int $width - Width of the image
- * @param int $height - Height of the image
- *
- * @return string
*/
- public function getBookCover($width = 440, $height = 250)
+ public function getBookCover(int $width = 440, int $height = 250): string
{
$default = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
- if (!$this->image_id) {
+ if (!$this->image_id || !$this->cover) {
return $default;
}
try {
- $cover = $this->cover ? url($this->cover->getThumb($width, $height, false)) : $default;
+ return $this->cover->getThumb($width, $height, false) ?? $default;
} catch (Exception $err) {
- $cover = $default;
+ return $default;
}
-
- return $cover;
}
/**
diff --git a/app/Entities/Models/Bookshelf.php b/app/Entities/Models/Bookshelf.php
index ad52d9d37..4b44025a4 100644
--- a/app/Entities/Models/Bookshelf.php
+++ b/app/Entities/Models/Bookshelf.php
@@ -3,6 +3,7 @@
namespace BookStack\Entities\Models;
use BookStack\Uploads\Image;
+use Exception;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\BelongsToMany;
@@ -49,28 +50,21 @@ class Bookshelf extends Entity implements HasCoverImage
}
/**
- * Returns BookShelf cover image, if cover does not exists return default cover image.
- *
- * @param int $width - Width of the image
- * @param int $height - Height of the image
- *
- * @return string
+ * Returns shelf cover image, if cover not exists return default cover image.
*/
- public function getBookCover($width = 440, $height = 250)
+ public function getBookCover(int $width = 440, int $height = 250): string
{
// TODO - Make generic, focused on books right now, Perhaps set-up a better image
$default = 'data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==';
- if (!$this->image_id) {
+ if (!$this->image_id || !$this->cover) {
return $default;
}
try {
- $cover = $this->cover ? url($this->cover->getThumb($width, $height, false)) : $default;
- } catch (\Exception $err) {
- $cover = $default;
+ return $this->cover->getThumb($width, $height, false) ?? $default;
+ } catch (Exception $err) {
+ return $default;
}
-
- return $cover;
}
/**
diff --git a/app/Uploads/Image.php b/app/Uploads/Image.php
index 5291dac05..5e197e750 100644
--- a/app/Uploads/Image.php
+++ b/app/Uploads/Image.php
@@ -45,13 +45,14 @@ class Image extends Model
}
/**
- * Get an (already existing) thumbnail for this image.
+ * Get a thumbnail URL for this image.
+ * Attempts to generate the thumbnail if not already existing.
*
* @throws \Exception
*/
public function getThumb(?int $width, ?int $height, bool $keepRatio = false): ?string
{
- return app()->make(ImageService::class)->getThumbnail($this, $width, $height, $keepRatio);
+ return app()->make(ImageService::class)->getThumbnail($this, $width, $height, $keepRatio, false, true);
}
/**
diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php
index 5db8defd3..c7e4aefad 100644
--- a/app/Uploads/ImageService.php
+++ b/app/Uploads/ImageService.php
@@ -21,23 +21,18 @@ use Intervention\Image\Exception\NotSupportedException;
use Intervention\Image\Image as InterventionImage;
use Intervention\Image\ImageManager;
use League\Flysystem\WhitespacePathNormalizer;
-use Psr\SimpleCache\InvalidArgumentException;
use Symfony\Component\HttpFoundation\File\UploadedFile;
use Symfony\Component\HttpFoundation\StreamedResponse;
class ImageService
{
- protected ImageManager $imageTool;
- protected Cache $cache;
- protected FilesystemManager $fileSystem;
-
protected static array $supportedExtensions = ['jpg', 'jpeg', 'png', 'gif', 'webp'];
- public function __construct(ImageManager $imageTool, FilesystemManager $fileSystem, Cache $cache)
- {
- $this->imageTool = $imageTool;
- $this->fileSystem = $fileSystem;
- $this->cache = $cache;
+ public function __construct(
+ protected ImageManager $imageTool,
+ protected FilesystemManager $fileSystem,
+ protected Cache $cache
+ ) {
}
/**
@@ -206,7 +201,7 @@ class ImageService
* Save image data for the given path in the public space, if possible,
* for the provided storage mechanism.
*/
- protected function saveImageDataInPublicSpace(Storage $storage, string $path, string $data)
+ protected function saveImageDataInPublicSpace(Storage $storage, string $path, string $data): void
{
$storage->put($path, $data);
@@ -269,8 +264,14 @@ class ImageService
*
* @throws Exception
*/
- public function getThumbnail(Image $image, ?int $width, ?int $height, bool $keepRatio = false, bool $shouldCreate = false): ?string
- {
+ public function getThumbnail(
+ Image $image,
+ ?int $width,
+ ?int $height,
+ bool $keepRatio = false,
+ bool $shouldCreate = false,
+ bool $canCreate = false,
+ ): ?string {
// Do not resize GIF images where we're not cropping
if ($keepRatio && $this->isGif($image)) {
return $this->getPublicUrl($image->path);
@@ -305,7 +306,7 @@ class ImageService
return $this->getPublicUrl($image->path);
}
- if (!$shouldCreate) {
+ if (!$shouldCreate && !$canCreate) {
return null;
}
@@ -559,7 +560,7 @@ class ImageService
// Check the image file exists
&& $disk->exists($imagePath)
// Check the file is likely an image file
- && strpos($disk->mimeType($imagePath), 'image/') === 0;
+ && str_starts_with($disk->mimeType($imagePath), 'image/');
}
/**
@@ -568,14 +569,14 @@ class ImageService
*/
protected function checkUserHasAccessToRelationOfImageAtPath(string $path): bool
{
- if (strpos($path, '/uploads/images/') === 0) {
+ if (str_starts_with($path, '/uploads/images/')) {
$path = substr($path, 15);
}
// Strip thumbnail element from path if existing
$originalPathSplit = array_filter(explode('/', $path), function (string $part) {
- $resizedDir = (strpos($part, 'thumbs-') === 0 || strpos($part, 'scaled-') === 0);
- $missingExtension = strpos($part, '.') === false;
+ $resizedDir = (str_starts_with($part, 'thumbs-') || str_starts_with($part, 'scaled-'));
+ $missingExtension = !str_contains($part, '.');
return !($resizedDir && $missingExtension);
});
@@ -641,9 +642,9 @@ class ImageService
$url = ltrim(trim($url), '/');
// Handle potential relative paths
- $isRelative = strpos($url, 'http') !== 0;
+ $isRelative = !str_starts_with($url, 'http');
if ($isRelative) {
- if (strpos(strtolower($url), 'uploads/images') === 0) {
+ if (str_starts_with(strtolower($url), 'uploads/images')) {
return trim($url, '/');
}
@@ -658,7 +659,7 @@ class ImageService
foreach ($potentialHostPaths as $potentialBasePath) {
$potentialBasePath = strtolower($potentialBasePath);
- if (strpos(strtolower($url), $potentialBasePath) === 0) {
+ if (str_starts_with(strtolower($url), $potentialBasePath)) {
return 'uploads/images/' . trim(substr($url, strlen($potentialBasePath)), '/');
}
}
@@ -679,7 +680,7 @@ class ImageService
// region-based url will be used to prevent http issues.
if (!$storageUrl && config('filesystems.images') === 's3') {
$storageDetails = config('filesystems.disks.s3');
- if (strpos($storageDetails['bucket'], '.') === false) {
+ if (!str_contains($storageDetails['bucket'], '.')) {
$storageUrl = 'https://' . $storageDetails['bucket'] . '.s3.amazonaws.com';
} else {
$storageUrl = 'https://s3-' . $storageDetails['region'] . '.amazonaws.com/' . $storageDetails['bucket'];
diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php
index 39236c7e4..5bd308ae8 100644
--- a/app/Users/Models/User.php
+++ b/app/Users/Models/User.php
@@ -244,7 +244,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon
}
try {
- $avatar = $this->avatar ? url($this->avatar->getThumb($size, $size, false)) : $default;
+ $avatar = $this->avatar?->getThumb($size, $size, false) ?? $default;
} catch (Exception $err) {
$avatar = $default;
}