From 4360da03d414cc926dde3d79e22e6e35f85838e1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 1 Nov 2021 11:17:30 +0000 Subject: [PATCH] Ran a pass through image and attachment routes Added some stronger types, formatting changes and simplifications along the way. --- app/Actions/CommentRepo.php | 4 +- app/Http/Controllers/AttachmentController.php | 5 ++- .../Images/DrawioImageController.php | 5 +-- app/Uploads/ImageRepo.php | 29 +++++--------- app/Uploads/ImageService.php | 38 ++++++------------- 5 files changed, 28 insertions(+), 53 deletions(-) diff --git a/app/Actions/CommentRepo.php b/app/Actions/CommentRepo.php index 85fb6498a..8121dfc5c 100644 --- a/app/Actions/CommentRepo.php +++ b/app/Actions/CommentRepo.php @@ -66,13 +66,13 @@ class CommentRepo /** * Delete a comment from the system. */ - public function delete(Comment $comment) + public function delete(Comment $comment): void { $comment->delete(); } /** - * Convert the given comment markdown text to HTML. + * Convert the given comment Markdown to HTML. */ public function commentToHtml(string $commentText): string { diff --git a/app/Http/Controllers/AttachmentController.php b/app/Http/Controllers/AttachmentController.php index 56503a694..477640b0a 100644 --- a/app/Http/Controllers/AttachmentController.php +++ b/app/Http/Controllers/AttachmentController.php @@ -68,6 +68,7 @@ class AttachmentController extends Controller 'file' => 'required|file', ]); + /** @var Attachment $attachment */ $attachment = Attachment::query()->findOrFail($attachmentId); $this->checkOwnablePermission('view', $attachment->page); $this->checkOwnablePermission('page-update', $attachment->page); @@ -86,11 +87,10 @@ class AttachmentController extends Controller /** * Get the update form for an attachment. - * - * @return \Illuminate\Contracts\Foundation\Application|\Illuminate\Contracts\View\Factory|\Illuminate\View\View */ public function getUpdateForm(string $attachmentId) { + /** @var Attachment $attachment */ $attachment = Attachment::query()->findOrFail($attachmentId); $this->checkOwnablePermission('page-update', $attachment->page); @@ -173,6 +173,7 @@ class AttachmentController extends Controller /** * Get the attachments for a specific page. + * @throws NotFoundException */ public function listForPage(int $pageId) { diff --git a/app/Http/Controllers/Images/DrawioImageController.php b/app/Http/Controllers/Images/DrawioImageController.php index d99bb8e6f..b8e4546ff 100644 --- a/app/Http/Controllers/Images/DrawioImageController.php +++ b/app/Http/Controllers/Images/DrawioImageController.php @@ -67,13 +67,12 @@ class DrawioImageController extends Controller public function getAsBase64($id) { $image = $this->imageRepo->getById($id); - $page = $image->getPage(); - if ($image === null || $image->type !== 'drawio' || !userCan('page-view', $page)) { + if (is_null($image) || $image->type !== 'drawio' || !userCan('page-view', $image->getPage())) { return $this->jsonError('Image data could not be found'); } $imageData = $this->imageRepo->getImageData($image); - if ($imageData === null) { + if (is_null($imageData)) { return $this->jsonError('Image data could not be found'); } diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 4d46bb56d..67297f308 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -17,10 +17,7 @@ class ImageRepo /** * ImageRepo constructor. */ - public function __construct( - ImageService $imageService, - PermissionService $permissionService - ) { + public function __construct(ImageService $imageService, PermissionService $permissionService) { $this->imageService = $imageService; $this->restrictionService = $permissionService; } @@ -43,7 +40,7 @@ class ImageRepo $hasMore = count($images) > $pageSize; $returnImages = $images->take($pageSize); - $returnImages->each(function ($image) { + $returnImages->each(function (Image $image) { $this->loadThumbs($image); }); @@ -96,6 +93,7 @@ class ImageRepo int $uploadedTo = null, string $search = null ): array { + /** @var Page $contextPage */ $contextPage = Page::visible()->findOrFail($uploadedTo); $parentFilter = null; @@ -131,7 +129,7 @@ class ImageRepo * * @throws ImageUploadException */ - public function saveNewFromData(string $imageName, string $imageData, string $type, int $uploadedTo = 0) + public function saveNewFromData(string $imageName, string $imageData, string $type, int $uploadedTo = 0): Image { $image = $this->imageService->saveNew($imageName, $imageData, $type, $uploadedTo); $this->loadThumbs($image); @@ -140,13 +138,13 @@ class ImageRepo } /** - * Save a drawing the the database. + * Save a drawing in the database. * * @throws ImageUploadException */ public function saveDrawing(string $base64Uri, int $uploadedTo): Image { - $name = 'Drawing-' . strval(user()->id) . '-' . strval(time()) . '.png'; + $name = 'Drawing-' . user()->id . '-' . time() . '.png'; return $this->imageService->saveNewFromBase64Uri($base64Uri, $name, 'drawio', $uploadedTo); } @@ -154,7 +152,6 @@ class ImageRepo /** * Update the details of an image via an array of properties. * - * @throws ImageUploadException * @throws Exception */ public function updateImageDetails(Image $image, $updateDetails): Image @@ -171,13 +168,11 @@ class ImageRepo * * @throws Exception */ - public function destroyImage(Image $image = null): bool + public function destroyImage(Image $image = null): void { if ($image) { $this->imageService->destroy($image); } - - return true; } /** @@ -185,7 +180,7 @@ class ImageRepo * * @throws Exception */ - public function destroyByType(string $imageType) + public function destroyByType(string $imageType): void { $images = Image::query()->where('type', '=', $imageType)->get(); foreach ($images as $image) { @@ -195,10 +190,8 @@ class ImageRepo /** * Load thumbnails onto an image object. - * - * @throws Exception */ - public function loadThumbs(Image $image) + public function loadThumbs(Image $image): void { $image->setAttribute('thumbs', [ 'gallery' => $this->getThumbnail($image, 150, 150, false), @@ -210,10 +203,8 @@ class ImageRepo * Get the thumbnail for an image. * If $keepRatio is true only the width will be used. * Checks the cache then storage to avoid creating / accessing the filesystem on every check. - * - * @throws Exception */ - protected function getThumbnail(Image $image, ?int $width = 220, ?int $height = 220, bool $keepRatio = false): ?string + protected function getThumbnail(Image $image, ?int $width, ?int $height, bool $keepRatio): ?string { try { return $this->imageService->getThumbnail($image, $width, $height, $keepRatio); diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index 4cd818bcc..eb2fc57b8 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -15,6 +15,8 @@ use Illuminate\Support\Str; use Intervention\Image\Exception\NotSupportedException; use Intervention\Image\ImageManager; use League\Flysystem\Util; +use Log; +use Psr\SimpleCache\InvalidArgumentException; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\StreamedResponse; @@ -156,7 +158,7 @@ class ImageService try { $this->saveImageDataInPublicSpace($storage, $this->adjustPathForStorageDisk($fullPath, $type), $imageData); } catch (Exception $e) { - \Log::error('Error when attempting image upload:' . $e->getMessage()); + Log::error('Error when attempting image upload:' . $e->getMessage()); throw new ImageUploadException(trans('errors.path_not_writable', ['filePath' => $fullPath])); } @@ -230,18 +232,10 @@ class ImageService * Get the thumbnail for an image. * If $keepRatio is true only the width will be used. * Checks the cache then storage to avoid creating / accessing the filesystem on every check. - * - * @param Image $image - * @param int $width - * @param int $height - * @param bool $keepRatio - * * @throws Exception - * @throws ImageUploadException - * - * @return string + * @throws InvalidArgumentException */ - public function getThumbnail(Image $image, $width = 220, $height = 220, $keepRatio = false) + public function getThumbnail(Image $image, ?int $width, ?int $height, bool $keepRatio = false): string { if ($keepRatio && $this->isGif($image)) { return $this->getPublicUrl($image->path); @@ -269,27 +263,16 @@ class ImageService } /** - * Resize image data. - * - * @param string $imageData - * @param int $width - * @param int $height - * @param bool $keepRatio + * Resize the image of given data to the specified size, and return the new image data. * * @throws ImageUploadException - * - * @return string */ - protected function resizeImage(string $imageData, $width = 220, $height = null, bool $keepRatio = true) + protected function resizeImage(string $imageData, ?int $width, ?int $height, bool $keepRatio): string { try { $thumb = $this->imageTool->make($imageData); - } catch (Exception $e) { - if ($e instanceof ErrorException || $e instanceof NotSupportedException) { - throw new ImageUploadException(trans('errors.cannot_create_thumbs')); - } - - throw $e; + } catch (ErrorException | NotSupportedException $e) { + throw new ImageUploadException(trans('errors.cannot_create_thumbs')); } if ($keepRatio) { @@ -523,7 +506,7 @@ class ImageService */ private function getPublicUrl(string $filePath): string { - if ($this->storageUrl === null) { + if (is_null($this->storageUrl)) { $storageUrl = config('filesystems.url'); // Get the standard public s3 url if s3 is set as storage type @@ -537,6 +520,7 @@ class ImageService $storageUrl = 'https://s3-' . $storageDetails['region'] . '.amazonaws.com/' . $storageDetails['bucket']; } } + $this->storageUrl = $storageUrl; }