From bd9047f2d103c9c3b8b676167e731b5d57652c55 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Fri, 17 Jun 2022 13:53:01 +0100 Subject: [PATCH 01/13] Added type hint --- modules/system/classes/MediaLibrary.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/system/classes/MediaLibrary.php b/modules/system/classes/MediaLibrary.php index f1488a2e0d..9bf4eaadb5 100644 --- a/modules/system/classes/MediaLibrary.php +++ b/modules/system/classes/MediaLibrary.php @@ -8,6 +8,7 @@ use Request; use Url; use Winter\Storm\Filesystem\Definitions as FileDefinitions; +use Illuminate\Filesystem\FilesystemAdapter; use ApplicationException; use SystemException; @@ -774,7 +775,7 @@ protected function filterItemList(&$itemList, $filter) * communicating with the remote storage. * @return mixed Returns the storage disk object. */ - public function getStorageDisk() + public function getStorageDisk(): FilesystemAdapter { if ($this->storageDisk) { return $this->storageDisk; From 4c9c54f186d2f5a5f0d6585cc1242c58464e7e90 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Fri, 17 Jun 2022 13:53:38 +0100 Subject: [PATCH 02/13] Updated typehint in docblock --- modules/backend/traits/SessionMaker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/backend/traits/SessionMaker.php b/modules/backend/traits/SessionMaker.php index d327cd0c72..2ca8340e66 100644 --- a/modules/backend/traits/SessionMaker.php +++ b/modules/backend/traits/SessionMaker.php @@ -17,7 +17,7 @@ trait SessionMaker /** * Saves a widget related key/value pair in to session data. * @param string $key Unique key for the data store. - * @param string $value The value to store. + * @param mixed $value The value to store. * @return void */ protected function putSession($key, $value) From e8e88b8c873faf003b0b71f3e1f4625e67b6aa3b Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Fri, 17 Jun 2022 14:26:26 +0100 Subject: [PATCH 03/13] Added make helper and implemented crop support --- modules/system/classes/ImageResizer.php | 108 +++++++++++++++++++++++- 1 file changed, 107 insertions(+), 1 deletion(-) diff --git a/modules/system/classes/ImageResizer.php b/modules/system/classes/ImageResizer.php index 71738f00f2..0360d01253 100644 --- a/modules/system/classes/ImageResizer.php +++ b/modules/system/classes/ImageResizer.php @@ -102,6 +102,29 @@ public function __construct($image, $width = 0, $height = 0, $options = []) $this->options = array_merge($this->getDefaultOptions(), $options); } + /** + * A simple static method for resizing an image and receiving the output path + * + * @param mixed $image + * @param int|float $width + * @param int|float $height + * @param array $options + * @return string + * @throws Exception + */ + public static function make(mixed $image, int|float $width = 0, int|float $height = 0, array $options = []): string + { + $mode = $options['mode'] === 'crop' ? 'crop' : 'resize'; + + if (!in_array($mode, ['resize', 'crop'])) { + throw new \ApplicationException('invalid mode passed to resizer'); + } + + $resizer = new static($image, $width, $height, $options); + $resizer->{$mode}(); + return $resizer->getPathToResizedImage(); + } + /** * Get the default options for the resizer * @@ -231,7 +254,7 @@ public function getConfig() $disk->setPathPrefix($realPath); } } - + // Include last modified time to tie generated images to the source image $mtime = $disk->lastModified($this->image['path']); @@ -346,6 +369,89 @@ public function resize() } } + public function crop() + { + if ($this->isResized()) { + return; + } + + // Get the details for the target image + list($disk, $path) = $this->getTargetDetails(); + + // Copy the image to be resized to the temp directory + $tempPath = $this->getLocalTempPath(); + + try { + /** + * @event system.resizer.processCrop + * Halting event that enables replacement of the resizing process. There should only ever be + * one listener handling this event per project at most, as other listeners would be ignored. + * + * Example usage: + * + * Event::listen('system.resizer.processCrop', function ((\System\Classes\ImageResizer) $resizer, (string) $localTempPath) { + * // Get the resizing configuration + * $config = $resizer->getConfig(); + * + * // Resize the image + * $resizedImageContents = My\Custom\Resizer::crop($localTempPath, $config['width], $config['height'], $config['options']); + * + * // Place the resized image in the correct location for the resizer to finish processing it + * file_put_contents($localTempPath, $resizedImageContents); + * + * // Prevent any other resizing replacer logic from running + * return true; + * }); + * + */ + $processed = Event::fire('system.resizer.processCrop', [$this, $tempPath], true); + if (!$processed) { + // Process the crop with the default image resizer + DefaultResizer::open($tempPath) + ->crop( + $this->options['offset'][0], + $this->options['offset'][1], + $this->width, + $this->height + ) + ->save($tempPath); + } + + /** + * @event system.resizer.afterCrop + * Enables post processing of resized images after they've been resized before the + * resizing process is finalized (ex. adding watermarks, further optimizing, etc) + * + * Example usage: + * + * Event::listen('system.resizer.afterCrop', function ((\System\Classes\ImageResizer) $resizer, (string) $localTempPath) { + * // Get the resized image data + * $croppedImageContents = file_get_contents($localTempPath); + * + * // Post process the image + * $processedContents = TinyPNG::optimize($croppedImageContents); + * + * // Place the processed image in the correct location for the resizer to finish processing it + * file_put_contents($localTempPath, $processedContents); + * }); + * + */ + Event::fire('system.resizer.afterCrop', [$this, $tempPath]); + + // Store the resized image + $disk->put($path, file_get_contents($tempPath)); + + // Clean up + unlink($tempPath); + } catch (Exception $ex) { + // Clean up in case of any issues + unlink($tempPath); + + // Pass the exception up + throw $ex; + } + } + /** * Define the internal working path, override this method to define. */ From 319cb90f10d16a7415864460937e8b373f883872 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Fri, 17 Jun 2022 14:28:55 +0100 Subject: [PATCH 04/13] Reworked thumbnail and cropping to use the ImageResizer --- modules/backend/widgets/MediaManager.php | 570 +++++------------- .../partials/_image-crop-popup-body.php | 3 +- 2 files changed, 144 insertions(+), 429 deletions(-) diff --git a/modules/backend/widgets/MediaManager.php b/modules/backend/widgets/MediaManager.php index 6e49b8b779..c67b655301 100644 --- a/modules/backend/widgets/MediaManager.php +++ b/modules/backend/widgets/MediaManager.php @@ -1,5 +1,6 @@ prepareVars(); return [ - '#'.$this->getId('item-list') => $this->makePartial('item-list'), - '#'.$this->getId('folder-path') => $this->makePartial('folder-path') + '#' . $this->getId('item-list') => $this->makePartial('item-list'), + '#' . $this->getId('folder-path') => $this->makePartial('folder-path') ]; } @@ -185,33 +187,17 @@ public function onGenerateThumbnails() */ public function onGetSidebarThumbnail() { - $path = Input::get('path'); + $path = MediaLibrary::validatePath(Input::get('path')); $lastModified = Input::get('lastModified'); - $thumbnailParams = $this->getThumbnailParams(); - $thumbnailParams['width'] = 300; - $thumbnailParams['height'] = 255; - $thumbnailParams['mode'] = 'auto'; - - $path = MediaLibrary::validatePath($path); - if (!is_numeric($lastModified)) { throw new ApplicationException('Invalid input data'); } - /* - * If the thumbnail file exists, just return the thumbnail markup, - * otherwise generate a new thumbnail. - */ - $thumbnailPath = $this->thumbnailExists($thumbnailParams, $path, $lastModified); - if ($thumbnailPath) { - return [ - 'markup' => $this->makePartial('thumbnail-image', [ - 'isError' => $this->thumbnailIsError($thumbnailPath), - 'imageUrl' => $this->getThumbnailImageUrl($thumbnailPath) - ]) - ]; - } + $thumbnailParams = $this->getThumbnailParams(); + $thumbnailParams['width'] = 300; + $thumbnailParams['height'] = 255; + $thumbnailParams['mode'] = 'auto'; $thumbnailInfo = $thumbnailParams; $thumbnailInfo['path'] = $path; @@ -236,9 +222,9 @@ public function onChangeView() $this->prepareVars(); return [ - '#'.$this->getId('item-list') => $this->makePartial('item-list'), - '#'.$this->getId('folder-path') => $this->makePartial('folder-path'), - '#'.$this->getId('view-mode-buttons') => $this->makePartial('view-mode-buttons') + '#' . $this->getId('item-list') => $this->makePartial('item-list'), + '#' . $this->getId('folder-path') => $this->makePartial('folder-path'), + '#' . $this->getId('view-mode-buttons') => $this->makePartial('view-mode-buttons') ]; } @@ -257,9 +243,9 @@ public function onSetFilter() $this->prepareVars(); return [ - '#'.$this->getId('item-list') => $this->makePartial('item-list'), - '#'.$this->getId('folder-path') => $this->makePartial('folder-path'), - '#'.$this->getId('filters') => $this->makePartial('filters') + '#' . $this->getId('item-list') => $this->makePartial('item-list'), + '#' . $this->getId('folder-path') => $this->makePartial('folder-path'), + '#' . $this->getId('filters') => $this->makePartial('filters') ]; } @@ -280,8 +266,8 @@ public function onSetSorting() $this->prepareVars(); return [ - '#'.$this->getId('item-list') => $this->makePartial('item-list'), - '#'.$this->getId('folder-path') => $this->makePartial('folder-path') + '#' . $this->getId('item-list') => $this->makePartial('item-list'), + '#' . $this->getId('folder-path') => $this->makePartial('folder-path') ]; } @@ -378,7 +364,7 @@ public function onDeleteItem() $this->prepareVars(); return [ - '#'.$this->getId('item-list') => $this->makePartial('item-list') + '#' . $this->getId('item-list') => $this->makePartial('item-list') ]; } @@ -543,7 +529,7 @@ public function onCreateFolder() $this->prepareVars(); return [ - '#'.$this->getId('item-list') => $this->makePartial('item-list') + '#' . $this->getId('item-list') => $this->makePartial('item-list') ]; } @@ -700,25 +686,23 @@ public function onLoadPopup() /** * Load image for cropping AJAX handler - * @return array + * @return string */ - public function onLoadImageCropPopup() + public function onLoadImageCropPopup(): string { $this->abortIfReadOnly(); $path = Input::get('path'); $path = MediaLibrary::validatePath($path); - $cropSessionKey = md5(FormHelper::getSessionKey()); $selectionParams = $this->getSelectionParams(); - $urlAndSize = $this->getCropEditImageUrlAndSize($path, $cropSessionKey); + $urlAndSize = $this->getCropEditImageUrlAndSize($path); $width = $urlAndSize['dimensions'][0]; $height = $urlAndSize['dimensions'][1] ?: 1; $this->vars['currentSelectionMode'] = $selectionParams['mode']; $this->vars['currentSelectionWidth'] = $selectionParams['width']; $this->vars['currentSelectionHeight'] = $selectionParams['height']; - $this->vars['cropSessionKey'] = $cropSessionKey; $this->vars['imageUrl'] = $urlAndSize['url']; $this->vars['dimensions'] = $urlAndSize['dimensions']; $this->vars['originalRatio'] = round($width / $height, 5); @@ -734,13 +718,6 @@ public function onLoadImageCropPopup() public function onEndCroppingSession() { $this->abortIfReadOnly(); - - $cropSessionKey = Input::get('cropSessionKey'); - if (!preg_match('/^[0-9a-z]+$/', $cropSessionKey)) { - throw new ApplicationException('Invalid input data'); - } - - $this->removeCropEditDir($cropSessionKey); } /** @@ -751,25 +728,14 @@ public function onCropImage() { $this->abortIfReadOnly(); - $imageSrcPath = trim(Input::get('img')); $selectionData = Input::get('selection'); - $cropSessionKey = Input::get('cropSessionKey'); - $path = Input::get('path'); - $path = MediaLibrary::validatePath($path); - - if (!strlen($imageSrcPath)) { - throw new ApplicationException('Invalid input data'); - } - - if (!preg_match('/^[0-9a-z]+$/', $cropSessionKey)) { - throw new ApplicationException('Invalid input data'); - } + $path = MediaLibrary::validatePath(Input::get('path')); if (!is_array($selectionData)) { throw new ApplicationException('Invalid input data'); } - $result = $this->cropImage($imageSrcPath, $selectionData, $cropSessionKey, $path); + $result = $this->cropImage($path, $selectionData); $selectionMode = Input::get('selectionMode'); $selectionWidth = Input::get('selectionWidth'); @@ -1163,27 +1129,28 @@ protected function getViewMode() /** * Returns thumbnail parameters - * @param string $viewMode - * @return array */ - protected function getThumbnailParams($viewMode = null) + protected function getThumbnailParams(string $viewMode = null): array { $result = [ 'mode' => 'crop' ]; - if ($viewMode) { - if ($viewMode == self::VIEW_MODE_LIST) { - $result['width'] = 75; - $result['height'] = 75; - } - else { - $result['width'] = 165; - $result['height'] = 165; - } + if (!$viewMode) { + return $result; } - return $result; + if ($viewMode === self::VIEW_MODE_LIST) { + return array_merge($result, [ + 'width' => 75, + 'height' => 75 + ]); + } + + return array_merge($result, [ + 'width' => 165, + 'height' => 165 + ]); } /** @@ -1227,44 +1194,15 @@ protected function getThumbnailImageExtension($itemPath) /** * Returns the URL to a thumbnail - * @param string $imagePath - * @return string */ - protected function getThumbnailImageUrl($imagePath) + protected function getThumbnailImageUrl(string $imagePath): string { - return Url::to('/storage/temp'.$imagePath); - } - - /** - * Check if a thumbnail exists - * @param array|null $thumbnailParams - * @param string $itemPath - * @param int $lastModified - * @return bool - */ - protected function thumbnailExists($thumbnailParams, $itemPath, $lastModified) - { - $thumbnailPath = $this->getThumbnailImagePath($thumbnailParams, $itemPath, $lastModified); - - $fullPath = temp_path(ltrim($thumbnailPath, '/')); - - if (File::exists($fullPath)) { - return $thumbnailPath; - } - - return false; - } - - /** - * Check if a thumbnail has caused an error - * @param string $thumbnailPath - * @return bool - */ - protected function thumbnailIsError($thumbnailPath) - { - $fullPath = temp_path(ltrim($thumbnailPath, '/')); - - return hash_file('crc32', $fullPath) == $this->getBrokenImageHash(); + return Url::to( + rtrim( + Config::get('cms.storage.resized.path', ''), + Config::get('cms.storage.resized.folder', '') + ) . $imagePath + ); } /** @@ -1316,80 +1254,57 @@ protected function getPlaceholderId($item) * @param array|null $thumbnailParams * @return array */ - protected function generateThumbnail($thumbnailInfo, $thumbnailParams = null) + protected function generateThumbnail($thumbnailInfo, $thumbnailParams = null): array { - $tempFilePath = null; - $fullThumbnailPath = null; - $thumbnailPath = null; $markup = null; - try { - $path = $thumbnailInfo['path']; + $path = $thumbnailInfo['path']; - if ($this->isVector($path)) { - $markup = $this->makePartial('thumbnail-image', [ + if ($this->isVector($path) && ($id = $thumbnailInfo['id'])) { + return [ + 'id' => $id, + 'markup' => $this->makePartial('thumbnail-image', [ 'isError' => false, 'imageUrl' => Url::to(config('cms.storage.media.path') . $thumbnailInfo['path']) - ]); - } else { - /* - * Get and validate input data - */ - $width = $thumbnailInfo['width']; - $height = $thumbnailInfo['height']; - $lastModified = $thumbnailInfo['lastModified']; - - if (!is_numeric($width) || !is_numeric($height) || !is_numeric($lastModified)) { - throw new ApplicationException('Invalid input data'); - } - - if (!$thumbnailParams) { - $thumbnailParams = $this->getThumbnailParams(); - $thumbnailParams['width'] = $width; - $thumbnailParams['height'] = $height; - } - - $thumbnailPath = $this->getThumbnailImagePath($thumbnailParams, $path, $lastModified); - $fullThumbnailPath = temp_path(ltrim($thumbnailPath, '/')); - - /* - * Save the file locally - */ - $library = MediaLibrary::instance(); - $tempFilePath = $this->getLocalTempFilePath($path); - - if (!@File::put($tempFilePath, $library->get($path))) { - throw new SystemException('Error saving remote file to a temporary location'); - } + ]) + ]; + } - /* - * Resize the thumbnail and save to the thumbnails directory - */ - $this->resizeImage($fullThumbnailPath, $thumbnailParams, $tempFilePath); + try { + /* + * Get and validate input data + */ + $width = $thumbnailInfo['width']; + $height = $thumbnailInfo['height']; + $lastModified = $thumbnailInfo['lastModified']; - /* - * Delete the temporary file - */ - File::delete($tempFilePath); - $markup = $this->makePartial('thumbnail-image', [ - 'isError' => false, - 'imageUrl' => $this->getThumbnailImageUrl($thumbnailPath) - ]); - } - } catch (Exception $ex) { - if ($tempFilePath) { - File::delete($tempFilePath); + if (!is_numeric($width) || !is_numeric($height) || !is_numeric($lastModified)) { + throw new ApplicationException('Invalid input data'); } - if ($fullThumbnailPath) { - $this->copyBrokenImage($fullThumbnailPath); + if (!$thumbnailParams) { + $thumbnailParams = $this->getThumbnailParams(); + $thumbnailParams['width'] = $width; + $thumbnailParams['height'] = $height; } - $markup = $this->makePartial('thumbnail-image', ['isError' => true]); + /* + * Resize the thumbnail and save to the thumbnails directory + */ + + + $fullThumbnailPath = $this->resizeImage(MediaLibrary::url($path), $thumbnailParams); /* - * @todo We need to log all types of exceptions here + * Delete the temporary file */ + $markup = $this->makePartial('thumbnail-image', [ + 'isError' => false, + 'imageUrl' => $this->getThumbnailImageUrl($fullThumbnailPath) + ]); + } catch (\Throwable $ex) { + $markup = $this->makePartial('thumbnail-image', ['isError' => true]); + traceLog($ex->getMessage()); } @@ -1399,39 +1314,19 @@ protected function generateThumbnail($thumbnailInfo, $thumbnailParams = null) 'markup' => $markup ]; } + + return []; } /** * Resize an image - * @param string $fullThumbnailPath - * @param array $thumbnailParams - * @param string $tempFilePath - * @return void */ - protected function resizeImage($fullThumbnailPath, $thumbnailParams, $tempFilePath) + protected function resizeImage(string $image, array $params): string { - $thumbnailDir = dirname($fullThumbnailPath); - if ( - !File::isDirectory($thumbnailDir) - && File::makeDirectory($thumbnailDir, 0777, true) === false - ) { - throw new SystemException('Error creating thumbnail directory'); - } - $targetDimensions = $this->getTargetDimensions($thumbnailParams['width'], $thumbnailParams['height'], $tempFilePath); - - $targetWidth = $targetDimensions[0]; - $targetHeight = $targetDimensions[1]; - - Resizer::open($tempFilePath) - ->resize($targetWidth, $targetHeight, [ - 'mode' => $thumbnailParams['mode'], - 'offset' => [0, 0] - ]) - ->save($fullThumbnailPath) - ; - - File::chmod($fullThumbnailPath); + return ImageResizer::make($image, $params['width'], $params['height'], array_merge([ + 'mode' => 'exact' + ], $params)); } /** @@ -1440,7 +1335,7 @@ protected function resizeImage($fullThumbnailPath, $thumbnailParams, $tempFilePa */ protected function getBrokenImagePath() { - return __DIR__.'/mediamanager/assets/images/broken-thumbnail.gif'; + return __DIR__ . '/mediamanager/assets/images/broken-thumbnail.gif'; } /** @@ -1458,278 +1353,99 @@ protected function getBrokenImageHash() return $this->brokenImageHash = hash_file('crc32', $fullPath); } - /** - * Copy broken image to destination - * @param string $path - * @return void - */ - protected function copyBrokenImage($path) - { - try { - $thumbnailDir = dirname($path); - if (!File::isDirectory($thumbnailDir) && File::makeDirectory($thumbnailDir, 0777, true) === false) { - return; - } - - File::copy($this->getBrokenImagePath(), $path); - } - catch (Exception $ex) { - traceLog($ex->getMessage()); - } - } - - /** - * Get target dimensions - * @param int $width - * @param int $height - * @param string $originalImagePath - * @return void - */ - protected function getTargetDimensions($width, $height, $originalImagePath) - { - $originalDimensions = [$width, $height]; - - try { - $dimensions = getimagesize($originalImagePath); - if (!$dimensions) { - return $originalDimensions; - } - - if ($dimensions[0] > $width || $dimensions[1] > $height) { - return $originalDimensions; - } - - return $dimensions; - } - catch (Exception $ex) { - return $originalDimensions; - } - } - // // Cropping // - /** - * Returns the crop session working directory path - * @param string $cropSessionKey - * @return string - */ - protected function getCropSessionDirPath($cropSessionKey) - { - return $this->getThumbnailDirectory().'edit-crop-'.$cropSessionKey; - } - /** * Prepares an image for cropping and returns payload containing a URL * @param string $path - * @param string $cropSessionKey * @param array $params * @return array */ - protected function getCropEditImageUrlAndSize($path, $cropSessionKey, $params = null) + protected function getCropEditImageUrlAndSize($path, $params = null) { - $sessionDirectoryPath = $this->getCropSessionDirPath($cropSessionKey); - $fullSessionDirectoryPath = temp_path($sessionDirectoryPath); - $sessionDirectoryCreated = false; + $url = MediaLibrary::url($path); - if (!File::isDirectory($fullSessionDirectoryPath)) { - File::makeDirectory($fullSessionDirectoryPath, 0777, true, true); - $sessionDirectoryCreated = true; + if ($params) { + $url = $this->resizeImage($url, [ + 'mode' => 'exact', + 'width' => $params['width'], + 'height' => $params['height'], + ]); } - $tempFilePath = null; - - try { - $extension = pathinfo($path, PATHINFO_EXTENSION); - $library = MediaLibrary::instance(); - $originalThumbFileName = 'original.'.$extension; - - /* - * If the target dimensions are not provided, save the original image to the - * crop session directory and return its URL. - */ - if (!$params) { - $tempFilePath = $fullSessionDirectoryPath.'/'.$originalThumbFileName; - - if (!@File::put($tempFilePath, $library->get($path))) { - throw new SystemException('Error saving remote file to a temporary location.'); - } - - $url = $this->getThumbnailImageUrl($sessionDirectoryPath.'/'.$originalThumbFileName); - $dimensions = getimagesize($tempFilePath); - - return [ - 'url' => $url, - 'dimensions' => $dimensions - ]; - } - /* - * If the target dimensions are provided, resize the original image and - * return its URL and dimensions. - */ - - $originalFilePath = $fullSessionDirectoryPath.'/'.$originalThumbFileName; - if (!File::isFile($originalFilePath)) { - throw new SystemException('The original image is not found in the cropping session directory.'); - } - - $resizedThumbFileName = 'resized-'.$params['width'].'-'.$params['height'].'.'.$extension; - $tempFilePath = $fullSessionDirectoryPath.'/'.$resizedThumbFileName; - - Resizer::open($originalFilePath) - ->resize($params['width'], $params['height'], [ - 'mode' => 'exact' - ]) - ->save($tempFilePath) - ; - - $url = $this->getThumbnailImageUrl($sessionDirectoryPath.'/'.$resizedThumbFileName); - $dimensions = getimagesize($tempFilePath); - - return [ - 'url' => $url, - 'dimensions' => $dimensions - ]; - } - catch (Exception $ex) { - if ($sessionDirectoryCreated) { - @File::deleteDirectory($fullSessionDirectoryPath); - } - - if ($tempFilePath) { - File::delete($tempFilePath); - } - - throw $ex; - } - } - - /** - * Cleans up the directory used for cropping based on the session key - * @param string $cropSessionKey - * @return void - */ - protected function removeCropEditDir($cropSessionKey) - { - $sessionDirectoryPath = $this->getCropSessionDirPath($cropSessionKey); - $fullSessionDirectoryPath = temp_path($sessionDirectoryPath); - - if (File::isDirectory($fullSessionDirectoryPath)) { - @File::deleteDirectory($fullSessionDirectoryPath); - } + return [ + 'url' => $url, + 'dimensions' => str_starts_with($url, '/') + ? getimagesize(base_path($url)) + : getimagesize($url) + ]; } /** * Business logic to crop a media library image - * @param string $imageSrcPath - * @param string $selectionData - * @param string $cropSessionKey - * @param string $path - * @return array */ - protected function cropImage($imageSrcPath, $selectionData, $cropSessionKey, $path) + protected function cropImage(string $path, array $selectionData): array { - $originalFileName = basename($path); - - $path = rtrim(dirname($path), '/').'/'; - $fileName = basename($imageSrcPath); - - if ( - strpos($fileName, '..') !== false || - strpos($fileName, '/') !== false || - strpos($fileName, '\\') !== false - ) { - throw new SystemException('Invalid image file name.'); - } - - $selectionParams = ['x', 'y', 'w', 'h']; - - foreach ($selectionParams as $paramName) { - if (!array_key_exists($paramName, $selectionData)) { + foreach (['x', 'y', 'w', 'h'] as $key) { + if (!isset($selectionData[$key]) || !is_numeric($selectionData[$key])) { throw new SystemException('Invalid selection data.'); } - if (!is_numeric($selectionData[$paramName])) { - throw new SystemException('Invalid selection data.'); - } - - $selectionData[$paramName] = (int) $selectionData[$paramName]; + $selectionData[$key] = (int) $selectionData[$key]; } - $sessionDirectoryPath = $this->getCropSessionDirPath($cropSessionKey); - $fullSessionDirectoryPath = temp_path($sessionDirectoryPath); - - if (!File::isDirectory($fullSessionDirectoryPath)) { - throw new SystemException('The image editing session is not found.'); - } - - /* - * Find the image on the disk and resize it - */ - $imagePath = $fullSessionDirectoryPath.'/'.$fileName; - if (!File::isFile($imagePath)) { - throw new SystemException('The image is not found on the disk.'); - } - - $extension = pathinfo($originalFileName, PATHINFO_EXTENSION); - - $targetImageName = basename($originalFileName, '.'.$extension).'-' - .$selectionData['x'].'-' - .$selectionData['y'].'-' - .$selectionData['w'].'-' - .$selectionData['h'].'-'; - - $targetImageName .= time(); - $targetImageName .= '.'.$extension; - - $targetTmpPath = $fullSessionDirectoryPath.'/'.$targetImageName; - - /* - * Crop the image, otherwise copy original to target destination. - */ - if ($selectionData['w'] == 0 || $selectionData['h'] == 0) { - File::copy($imagePath, $targetTmpPath); - } - else { - Resizer::open($imagePath) - ->crop( - $selectionData['x'], - $selectionData['y'], - $selectionData['w'], - $selectionData['h'], - $selectionData['w'], - $selectionData['h'] - ) - ->save($targetTmpPath) - ; - } + $croppedPath = $this->resizeImage(MediaLibrary::url($path), [ + 'mode' => 'crop', + 'height' => $selectionData['h'], + 'width' => $selectionData['w'], + 'offset' => [ + $selectionData['x'], + $selectionData['y'] + ] + ]); - /* - * Upload the cropped file to the Library - */ - $targetFolder = $path.'cropped-images'; - $targetPath = $targetFolder.'/'.$targetImageName; + $targetPath = $this->getCroppedPath($path); - $library = MediaLibrary::instance(); - $library->put($targetPath, file_get_contents($targetTmpPath)); + // would be nice to have Resizer::disk()->get($image) + MediaLibrary::instance()->put( + $targetPath, + Storage::disk(Config::get('cms.storage.resized.disk'))->get($croppedPath) + ); return [ - 'publicUrl' => $library->getPathUrl($targetPath), + 'publicUrl' => MediaLibrary::url($targetPath), 'documentType' => MediaLibraryItem::FILE_TYPE_IMAGE, 'itemType' => MediaLibraryItem::TYPE_FILE, 'path' => $targetPath, - 'title' => $targetImageName, - 'folder' => $targetFolder + 'title' => basename($path), + 'folder' => dirname($path) ]; } + /** + * Get a path to save a cropped image as + */ + protected function getCroppedPath(string $path): string + { + $parts = (!str_contains(basename($path), '.')) + ? [$path, ''] + : array_map('strrev', array_reverse(explode('.', strrev($path), 2))); + + $i = 1; + + do { + $cropped = sprintf('%s_cropped_%d.%s', $parts[0], $i++, $parts[1]); + } while (MediaLibrary::instance()->exists($cropped)); + + return $cropped; + } + /** * Detect if image is vector graphic (SVG) - * @param string $path - * @return boolean */ - protected function isVector($path) + protected function isVector(string $path): bool { return (pathinfo($path, PATHINFO_EXTENSION) == 'svg'); } diff --git a/modules/backend/widgets/mediamanager/partials/_image-crop-popup-body.php b/modules/backend/widgets/mediamanager/partials/_image-crop-popup-body.php index 9771480bdf..95f4ee4d4a 100644 --- a/modules/backend/widgets/mediamanager/partials/_image-crop-popup-body.php +++ b/modules/backend/widgets/mediamanager/partials/_image-crop-popup-body.php @@ -27,7 +27,6 @@ class="btn btn-default no-margin-right"> - @@ -40,4 +39,4 @@ class="btn btn-default no-margin-right"> makePartial('resize-image-form') ?> - \ No newline at end of file + From 97ed76249dd512c63ac2166eb440eb11568a43b7 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Fri, 17 Jun 2022 16:53:29 +0100 Subject: [PATCH 05/13] Removed whitespace --- modules/backend/widgets/MediaManager.php | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/backend/widgets/MediaManager.php b/modules/backend/widgets/MediaManager.php index c67b655301..e0e4dc9c69 100644 --- a/modules/backend/widgets/MediaManager.php +++ b/modules/backend/widgets/MediaManager.php @@ -1323,7 +1323,6 @@ protected function generateThumbnail($thumbnailInfo, $thumbnailParams = null): a */ protected function resizeImage(string $image, array $params): string { - return ImageResizer::make($image, $params['width'], $params['height'], array_merge([ 'mode' => 'exact' ], $params)); From fbf176c8fbfd0c42a5c7ffbfb61fb58198b2072c Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 22 Jun 2022 19:11:48 -0600 Subject: [PATCH 06/13] Code review and sanity check --- modules/backend/widgets/MediaManager.php | 118 +++++++++++++---------- modules/system/classes/ImageResizer.php | 27 ++++-- 2 files changed, 85 insertions(+), 60 deletions(-) diff --git a/modules/backend/widgets/MediaManager.php b/modules/backend/widgets/MediaManager.php index e0e4dc9c69..cc29ba79f7 100644 --- a/modules/backend/widgets/MediaManager.php +++ b/modules/backend/widgets/MediaManager.php @@ -735,7 +735,43 @@ public function onCropImage() throw new ApplicationException('Invalid input data'); } - $result = $this->cropImage($path, $selectionData); + foreach (['x', 'y', 'w', 'h'] as $key) { + if (!isset($selectionData[$key]) || !is_numeric($selectionData[$key])) { + throw new SystemException('Invalid selection data.'); + } + + $selectionData[$key] = (int) $selectionData[$key]; + } + + $croppedPath = $this->cropImage(MediaLibrary::url($path), [ + 'height' => $selectionData['h'], + 'width' => $selectionData['w'], + 'offset' => [ + $selectionData['x'], + $selectionData['y'] + ] + ]); + + $targetPath = $this->getCroppedPath($path); + + // @TODO: Push this work out to the ImageResizer class more perhaps + // Something like ImageResizer::getStorageDisk() - caveat being that + // the method wouldn't always return the actual disk the resized image + // is stored on given the support for storing resized FileModel images + // on their original disk. + MediaLibrary::instance()->put( + $targetPath, + Storage::disk(Config::get('cms.storage.resized.disk'))->get($croppedPath) + ); + + $result = [ + 'publicUrl' => MediaLibrary::url($targetPath), + 'documentType' => MediaLibraryItem::FILE_TYPE_IMAGE, + 'itemType' => MediaLibraryItem::TYPE_FILE, + 'path' => $targetPath, + 'title' => basename($path), + 'folder' => dirname($path), + ]; $selectionMode = Input::get('selectionMode'); $selectionWidth = Input::get('selectionWidth'); @@ -754,11 +790,6 @@ public function onResizeImage() { $this->abortIfReadOnly(); - $cropSessionKey = Input::get('cropSessionKey'); - if (!preg_match('/^[0-9a-z]+$/', $cropSessionKey)) { - throw new ApplicationException('Invalid input data'); - } - $width = trim(Input::get('width')); if (!strlen($width) || !ctype_digit($width)) { throw new ApplicationException('Invalid input data'); @@ -777,7 +808,7 @@ public function onResizeImage() 'height' => $height ]; - return $this->getCropEditImageUrlAndSize($path, $cropSessionKey, $params); + return $this->getCropEditImageUrlAndSize($path, $params); } // @@ -1291,8 +1322,6 @@ protected function generateThumbnail($thumbnailInfo, $thumbnailParams = null): a /* * Resize the thumbnail and save to the thumbnails directory */ - - $fullThumbnailPath = $this->resizeImage(MediaLibrary::url($path), $thumbnailParams); /* @@ -1323,9 +1352,33 @@ protected function generateThumbnail($thumbnailInfo, $thumbnailParams = null): a */ protected function resizeImage(string $image, array $params): string { - return ImageResizer::make($image, $params['width'], $params['height'], array_merge([ - 'mode' => 'exact' - ], $params)); + return ImageResizer::processImage( + $image, + $params['width'], + $params['height'], + array_merge( + ['mode' => 'exact'], + $params + ), + ImageResizer::METHOD_RESIZE + ); + } + + /** + * Crop an image + */ + protected function cropImage(string $image, array $params): string + { + return ImageResizer::processImage( + $image, + $params['width'], + $params['height'], + array_merge( + ['mode' => 'exact'], + $params + ), + ImageResizer::METHOD_CROP + ); } /** @@ -1382,47 +1435,6 @@ protected function getCropEditImageUrlAndSize($path, $params = null) ]; } - /** - * Business logic to crop a media library image - */ - protected function cropImage(string $path, array $selectionData): array - { - foreach (['x', 'y', 'w', 'h'] as $key) { - if (!isset($selectionData[$key]) || !is_numeric($selectionData[$key])) { - throw new SystemException('Invalid selection data.'); - } - - $selectionData[$key] = (int) $selectionData[$key]; - } - - $croppedPath = $this->resizeImage(MediaLibrary::url($path), [ - 'mode' => 'crop', - 'height' => $selectionData['h'], - 'width' => $selectionData['w'], - 'offset' => [ - $selectionData['x'], - $selectionData['y'] - ] - ]); - - $targetPath = $this->getCroppedPath($path); - - // would be nice to have Resizer::disk()->get($image) - MediaLibrary::instance()->put( - $targetPath, - Storage::disk(Config::get('cms.storage.resized.disk'))->get($croppedPath) - ); - - return [ - 'publicUrl' => MediaLibrary::url($targetPath), - 'documentType' => MediaLibraryItem::FILE_TYPE_IMAGE, - 'itemType' => MediaLibraryItem::TYPE_FILE, - 'path' => $targetPath, - 'title' => basename($path), - 'folder' => dirname($path) - ]; - } - /** * Get a path to save a cropped image as */ diff --git a/modules/system/classes/ImageResizer.php b/modules/system/classes/ImageResizer.php index 0360d01253..f4294f49a9 100644 --- a/modules/system/classes/ImageResizer.php +++ b/modules/system/classes/ImageResizer.php @@ -44,10 +44,16 @@ class ImageResizer { /** - * @var string The cache key prefix for resizer configs + * The cache key prefix for resizer configs */ public const CACHE_PREFIX = 'system.resizer.'; + /** + * Available methods to use when processing images + */ + public const METHOD_RESIZE = 'resize'; + public const METHOD_CROP = 'crop'; + /** * @var array Available sources to get images from */ @@ -112,16 +118,20 @@ public function __construct($image, $width = 0, $height = 0, $options = []) * @return string * @throws Exception */ - public static function make(mixed $image, int|float $width = 0, int|float $height = 0, array $options = []): string + public static function processImage( + mixed $image, + int|float $width = 0, + int|float $height = 0, + array $options = [], + string $method = self::METHOD_RESIZE + ): string { - $mode = $options['mode'] === 'crop' ? 'crop' : 'resize'; - - if (!in_array($mode, ['resize', 'crop'])) { - throw new \ApplicationException('invalid mode passed to resizer'); + if (!in_array($method, [static::METHOD_RESIZE, static::METHOD_CROP])) { + throw new \ApplicationException('Invalid method passed to processImage'); } $resizer = new static($image, $width, $height, $options); - $resizer->{$mode}(); + $resizer->{$method}(); return $resizer->getPathToResizedImage(); } @@ -369,6 +379,9 @@ public function resize() } } + /** + * Process the crop request + */ public function crop() { if ($this->isResized()) { From 4e7bb7a63d78ff3d56cbf9282696e29a3729fdac Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 22 Jun 2022 19:16:51 -0600 Subject: [PATCH 07/13] Code review --- modules/system/classes/ImageResizer.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/modules/system/classes/ImageResizer.php b/modules/system/classes/ImageResizer.php index f4294f49a9..7ab938ab03 100644 --- a/modules/system/classes/ImageResizer.php +++ b/modules/system/classes/ImageResizer.php @@ -111,12 +111,7 @@ public function __construct($image, $width = 0, $height = 0, $options = []) /** * A simple static method for resizing an image and receiving the output path * - * @param mixed $image - * @param int|float $width - * @param int|float $height - * @param array $options - * @return string - * @throws Exception + * @throws ApplicationException If an invalid resize mode is passed to the the method. */ public static function processImage( mixed $image, @@ -397,7 +392,7 @@ public function crop() try { /** * @event system.resizer.processCrop - * Halting event that enables replacement of the resizing process. There should only ever be + * Halting event that enables replacement of the cropping process. There should only ever be * one listener handling this event per project at most, as other listeners would be ignored. * * Example usage: @@ -432,8 +427,8 @@ public function crop() /** * @event system.resizer.afterCrop - * Enables post processing of resized images after they've been resized before the - * resizing process is finalized (ex. adding watermarks, further optimizing, etc) + * Enables post processing of cropped images after they've been cropped before the + * cropping process is finalized (ex. adding watermarks, further optimizing, etc) * * Example usage: * From 77ffb6afce8a3a239197fd7b063c81aa08597945 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 22 Jun 2022 19:46:08 -0600 Subject: [PATCH 08/13] Clean up path deduplication --- modules/backend/widgets/MediaManager.php | 38 ++++++++++++++++-------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/modules/backend/widgets/MediaManager.php b/modules/backend/widgets/MediaManager.php index cc29ba79f7..cf022ac0b5 100644 --- a/modules/backend/widgets/MediaManager.php +++ b/modules/backend/widgets/MediaManager.php @@ -748,11 +748,19 @@ public function onCropImage() 'width' => $selectionData['w'], 'offset' => [ $selectionData['x'], - $selectionData['y'] - ] + $selectionData['y'], + ], ]); - $targetPath = $this->getCroppedPath($path); + // Generate the target path for the cropped image + $parts = pathinfo($path); + $targetPath = sprintf( + '%s/%s_cropped.%s', + $parts['dirname'], + $parts['filename'], + $parts['extension'], + ); + $targetPath = $this->deduplicatePath($targetPath); // @TODO: Push this work out to the ImageResizer class more perhaps // Something like ImageResizer::getStorageDisk() - caveat being that @@ -1436,21 +1444,25 @@ protected function getCropEditImageUrlAndSize($path, $params = null) } /** - * Get a path to save a cropped image as + * Process the provided path and add a suffix of _$int to prevent conflicts + * with existing paths */ - protected function getCroppedPath(string $path): string + protected function deduplicatePath(string $path): string { - $parts = (!str_contains(basename($path), '.')) - ? [$path, ''] - : array_map('strrev', array_reverse(explode('.', strrev($path), 2))); - + $parts = pathinfo($path); $i = 1; - do { - $cropped = sprintf('%s_cropped_%d.%s', $parts[0], $i++, $parts[1]); - } while (MediaLibrary::instance()->exists($cropped)); + while (MediaLibrary::instance()->exists($path)) { + $path = sprintf( + '%s/%s_%d.%s', + $parts['dirname'], + $parts['filename'], + $i++, + $parts['extension'] + ); + } - return $cropped; + return $path; } /** From 686b934afd5b418657feb73fb012189f1eb1f5a7 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Sun, 26 Jun 2022 22:18:05 +0100 Subject: [PATCH 09/13] Added fix for resizing image --- modules/backend/widgets/MediaManager.php | 33 +++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/modules/backend/widgets/MediaManager.php b/modules/backend/widgets/MediaManager.php index cf022ac0b5..274485f6c8 100644 --- a/modules/backend/widgets/MediaManager.php +++ b/modules/backend/widgets/MediaManager.php @@ -743,6 +743,10 @@ public function onCropImage() $selectionData[$key] = (int) $selectionData[$key]; } + if ($selectionData['h'] === 0 || $selectionData['w'] === 0) { + throw new ApplicationException('You must define a crop size before inserting'); + } + $croppedPath = $this->cropImage(MediaLibrary::url($path), [ 'height' => $selectionData['h'], 'width' => $selectionData['w'], @@ -811,12 +815,35 @@ public function onResizeImage() $path = Input::get('path'); $path = MediaLibrary::validatePath($path); - $params = [ + $croppedPath = $this->resizeImage(MediaLibrary::url($path), [ + 'mode' => 'exact', 'width' => $width, 'height' => $height - ]; + ]); + + $parts = pathinfo($path); + $targetPath = sprintf( + '%s/%s_resized.%s', + $parts['dirname'], + $parts['filename'], + $parts['extension'], + ); + $targetPath = $this->deduplicatePath($targetPath); - return $this->getCropEditImageUrlAndSize($path, $params); + // @TODO: Push this work out to the ImageResizer class more perhaps + // Something like ImageResizer::getStorageDisk() - caveat being that + // the method wouldn't always return the actual disk the resized image + // is stored on given the support for storing resized FileModel images + // on their original disk. + MediaLibrary::instance()->put( + $targetPath, + Storage::disk(Config::get('cms.storage.resized.disk'))->get($croppedPath) + ); + + return [ + 'url' => MediaLibrary::url($targetPath), + 'dimensions' => [$width, $height] + ]; } // From 59970b96a61a56b6aa4f3972bdf888a3626ed309 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 29 Jun 2022 10:31:22 +0100 Subject: [PATCH 10/13] Exposed methods for accessing the resized disk --- modules/system/classes/ImageResizer.php | 26 +++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/modules/system/classes/ImageResizer.php b/modules/system/classes/ImageResizer.php index 7ab938ab03..af06c92c97 100644 --- a/modules/system/classes/ImageResizer.php +++ b/modules/system/classes/ImageResizer.php @@ -535,6 +535,18 @@ public function getFileModel() return $this->fileModel; } + public static function getDefaultDisk() + { + return Storage::disk(Config::get('cms.storage.resized.disk', 'local')); + } + + public function getDisk(): \Illuminate\Contracts\Filesystem\Filesystem + { + return ($this->image['source'] === 'filemodel' && $fileModel = $this->getFileModel()) + ? $fileModel->getDisk() + : static::getDefaultDisk(); + } + /** * Get the details for the target image * @@ -543,14 +555,16 @@ public function getFileModel() protected function getTargetDetails() { if ($this->image['source'] === 'filemodel' && $fileModel = $this->getFileModel()) { - $disk = $fileModel->getDisk(); - $path = $fileModel->getDiskPath($fileModel->getThumbFilename($this->width, $this->height, $this->options)); - } else { - $disk = Storage::disk(Config::get('cms.storage.resized.disk', 'local')); - $path = $this->getPathToResizedImage(); + return [ + $this->getDisk(), + $fileModel->getDiskPath($fileModel->getThumbFilename($this->width, $this->height, $this->options)), + ]; } - return [$disk, $path]; + return [ + $this->getDisk(), + $this->getPathToResizedImage(), + ]; } /** From f79f2cb435cd20b32cfb0a74dbeb71aab91e97ca Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 29 Jun 2022 10:31:45 +0100 Subject: [PATCH 11/13] Fixed support for cropping from a resized image --- modules/backend/widgets/MediaManager.php | 97 ++++++++++++++---------- 1 file changed, 55 insertions(+), 42 deletions(-) diff --git a/modules/backend/widgets/MediaManager.php b/modules/backend/widgets/MediaManager.php index 274485f6c8..671d425f56 100644 --- a/modules/backend/widgets/MediaManager.php +++ b/modules/backend/widgets/MediaManager.php @@ -20,6 +20,7 @@ use Winter\Storm\Database\Attach\Resizer; use Winter\Storm\Filesystem\Definitions as FileDefinitions; use Form as FormHelper; +use Winter\Storm\Filesystem\Storage\Resized; /** * Media Manager widget. @@ -155,8 +156,8 @@ public function onGoToFolder() $this->prepareVars(); return [ - '#'.$this->getId('item-list') => $this->makePartial('item-list'), - '#'.$this->getId('folder-path') => $this->makePartial('folder-path') + '#' . $this->getId('item-list') => $this->makePartial('item-list'), + '#' . $this->getId('folder-path') => $this->makePartial('folder-path') ]; } @@ -177,7 +178,7 @@ public function onGenerateThumbnails() } return [ - 'generatedThumbnails'=>$result + 'generatedThumbnails' => $result ]; } @@ -301,8 +302,7 @@ public function onDeleteItem() * Add to bulk collection */ $filesToDelete[] = $path; - } - elseif ($type === MediaLibraryItem::TYPE_FOLDER) { + } elseif ($type === MediaLibraryItem::TYPE_FOLDER) { /* * Delete single folder */ @@ -406,7 +406,7 @@ public function onApplyName() $originalPath = Input::get('originalPath'); $originalPath = MediaLibrary::validatePath($originalPath); - $newPath = dirname($originalPath).'/'.$newName; + $newPath = dirname($originalPath) . '/' . $newName; $type = Input::get('type'); if ($type == MediaLibraryItem::TYPE_FILE) { @@ -440,8 +440,7 @@ public function onApplyName() * */ $this->fireSystemEvent('media.file.rename', [$originalPath, $newPath]); - } - else { + } else { /* * Move single folder */ @@ -490,7 +489,7 @@ public function onCreateFolder() $path = Input::get('path'); $path = MediaLibrary::validatePath($path); - $newFolderPath = $path.'/'.$name; + $newFolderPath = $path . '/' . $name; $library = MediaLibrary::instance(); @@ -554,10 +553,9 @@ public function onLoadMovePopup() if ($folder == '/') { $name = Lang::get('backend::lang.media.library'); - } - else { + } else { $segments = explode('/', $folder); - $name = str_repeat(' ', (count($segments)-1)*4).basename($folder); + $name = str_repeat(' ', (count($segments) - 1) * 4) . basename($folder); } $folderList[$path] = $name; @@ -603,7 +601,7 @@ public function onMoveItems() /* * Move a single file */ - $library->moveFile($path, $dest.'/'.basename($path)); + $library->moveFile($path, $dest . '/' . basename($path)); /** * @event media.file.move @@ -629,7 +627,7 @@ public function onMoveItems() /* * Move a single folder */ - $library->moveFolder($path, $dest.'/'.basename($path)); + $library->moveFolder($path, $dest . '/' . basename($path)); /** * @event media.folder.move @@ -656,7 +654,7 @@ public function onMoveItems() $this->prepareVars(); return [ - '#'.$this->getId('item-list') => $this->makePartial('item-list') + '#' . $this->getId('item-list') => $this->makePartial('item-list') ]; } @@ -729,7 +727,8 @@ public function onCropImage() $this->abortIfReadOnly(); $selectionData = Input::get('selection'); - $path = MediaLibrary::validatePath(Input::get('path')); + + [$file, $name, $disk] = $this->resolveImagePath(Input::get('img'), Input::get('path')); if (!is_array($selectionData)) { throw new ApplicationException('Invalid input data'); @@ -740,14 +739,14 @@ public function onCropImage() throw new SystemException('Invalid selection data.'); } - $selectionData[$key] = (int) $selectionData[$key]; + $selectionData[$key] = (int)$selectionData[$key]; } if ($selectionData['h'] === 0 || $selectionData['w'] === 0) { throw new ApplicationException('You must define a crop size before inserting'); } - $croppedPath = $this->cropImage(MediaLibrary::url($path), [ + $croppedPath = $this->cropImage($disk === 'resized' ? $file : MediaLibrary::url($file), [ 'height' => $selectionData['h'], 'width' => $selectionData['w'], 'offset' => [ @@ -757,11 +756,11 @@ public function onCropImage() ]); // Generate the target path for the cropped image - $parts = pathinfo($path); + $parts = pathinfo($name); $targetPath = sprintf( '%s/%s_cropped.%s', $parts['dirname'], - $parts['filename'], + preg_replace('/_cropped(_\d)?/', '', $parts['filename']), $parts['extension'], ); $targetPath = $this->deduplicatePath($targetPath); @@ -781,8 +780,8 @@ public function onCropImage() 'documentType' => MediaLibraryItem::FILE_TYPE_IMAGE, 'itemType' => MediaLibraryItem::TYPE_FILE, 'path' => $targetPath, - 'title' => basename($path), - 'folder' => dirname($path), + 'title' => basename($targetPath), + 'folder' => dirname($targetPath), ]; $selectionMode = Input::get('selectionMode'); @@ -794,6 +793,39 @@ public function onCropImage() return $result; } + /* + * This method resolves the img & path submit by the resizer and works what disk they belong to + */ + public function resolveImagePath(string $img, string $path): array + { + $disk = null; + $actual = $img; + + // if the image has been resized, we will be passed it's url in img + if (filter_var($actual, FILTER_VALIDATE_URL)) { + $actual = parse_url($actual)['path'] ?? ''; + } + + if (str_starts_with($actual, Config::get('cms.storage.resized.path'))) { + $actual = substr($actual, strlen(Config::get('cms.storage.resized.path'))); + $actual = Config::get('cms.storage.resized.folder') . (!str_starts_with($actual, '/') ? '/' : '') . $actual; + $disk = 'resized'; + } + + if (!ImageResizer::getDefaultDisk()->exists($actual)) { + $actual = $path; + $disk = 'media'; + } + + return [ + $disk === 'resized' + ? $img + : MediaLibrary::validatePath($actual), + MediaLibrary::validatePath($path), + $disk + ]; + } + /** * Resize image AJAX handler * @return array @@ -821,27 +853,8 @@ public function onResizeImage() 'height' => $height ]); - $parts = pathinfo($path); - $targetPath = sprintf( - '%s/%s_resized.%s', - $parts['dirname'], - $parts['filename'], - $parts['extension'], - ); - $targetPath = $this->deduplicatePath($targetPath); - - // @TODO: Push this work out to the ImageResizer class more perhaps - // Something like ImageResizer::getStorageDisk() - caveat being that - // the method wouldn't always return the actual disk the resized image - // is stored on given the support for storing resized FileModel images - // on their original disk. - MediaLibrary::instance()->put( - $targetPath, - Storage::disk(Config::get('cms.storage.resized.disk'))->get($croppedPath) - ); - return [ - 'url' => MediaLibrary::url($targetPath), + 'url' => $this->getThumbnailImageUrl($croppedPath), 'dimensions' => [$width, $height] ]; } From 1449146259d6ec14bc54d0dcbeec7f505c19648f Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 29 Jun 2022 12:03:23 +0100 Subject: [PATCH 12/13] Code style fix --- modules/system/classes/ImageResizer.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/system/classes/ImageResizer.php b/modules/system/classes/ImageResizer.php index af06c92c97..182b2097d5 100644 --- a/modules/system/classes/ImageResizer.php +++ b/modules/system/classes/ImageResizer.php @@ -119,8 +119,7 @@ public static function processImage( int|float $height = 0, array $options = [], string $method = self::METHOD_RESIZE - ): string - { + ): string { if (!in_array($method, [static::METHOD_RESIZE, static::METHOD_CROP])) { throw new \ApplicationException('Invalid method passed to processImage'); } From 397a5566d5e1727dc4d51eadff296a9f06d3f916 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 29 Jun 2022 15:43:21 -0600 Subject: [PATCH 13/13] Simplify path generation logic --- modules/backend/widgets/MediaManager.php | 92 ++++++++++-------------- modules/system/classes/ImageResizer.php | 90 +++++++++-------------- 2 files changed, 69 insertions(+), 113 deletions(-) diff --git a/modules/backend/widgets/MediaManager.php b/modules/backend/widgets/MediaManager.php index 671d425f56..ea29929c09 100644 --- a/modules/backend/widgets/MediaManager.php +++ b/modules/backend/widgets/MediaManager.php @@ -1,6 +1,5 @@ abortIfReadOnly(); $selectionData = Input::get('selection'); - - [$file, $name, $disk] = $this->resolveImagePath(Input::get('img'), Input::get('path')); + $sourceImageUrl = Input::get('img'); + $mediaItemPath = Input::get('path'); if (!is_array($selectionData)) { throw new ApplicationException('Invalid input data'); @@ -739,14 +738,14 @@ public function onCropImage() throw new SystemException('Invalid selection data.'); } - $selectionData[$key] = (int)$selectionData[$key]; + $selectionData[$key] = (int) $selectionData[$key]; } if ($selectionData['h'] === 0 || $selectionData['w'] === 0) { throw new ApplicationException('You must define a crop size before inserting'); } - $croppedPath = $this->cropImage($disk === 'resized' ? $file : MediaLibrary::url($file), [ + $croppedPath = $this->cropImage($sourceImageUrl, [ 'height' => $selectionData['h'], 'width' => $selectionData['w'], 'offset' => [ @@ -756,23 +755,12 @@ public function onCropImage() ]); // Generate the target path for the cropped image - $parts = pathinfo($name); - $targetPath = sprintf( - '%s/%s_cropped.%s', - $parts['dirname'], - preg_replace('/_cropped(_\d)?/', '', $parts['filename']), - $parts['extension'], - ); - $targetPath = $this->deduplicatePath($targetPath); + $targetPath = $this->deduplicatePath($mediaItemPath, '_cropped'); - // @TODO: Push this work out to the ImageResizer class more perhaps - // Something like ImageResizer::getStorageDisk() - caveat being that - // the method wouldn't always return the actual disk the resized image - // is stored on given the support for storing resized FileModel images - // on their original disk. + // Move the cropped image to the target path MediaLibrary::instance()->put( $targetPath, - Storage::disk(Config::get('cms.storage.resized.disk'))->get($croppedPath) + ImageResizer::getDefaultDisk()->get($croppedPath) ); $result = [ @@ -793,39 +781,6 @@ public function onCropImage() return $result; } - /* - * This method resolves the img & path submit by the resizer and works what disk they belong to - */ - public function resolveImagePath(string $img, string $path): array - { - $disk = null; - $actual = $img; - - // if the image has been resized, we will be passed it's url in img - if (filter_var($actual, FILTER_VALIDATE_URL)) { - $actual = parse_url($actual)['path'] ?? ''; - } - - if (str_starts_with($actual, Config::get('cms.storage.resized.path'))) { - $actual = substr($actual, strlen(Config::get('cms.storage.resized.path'))); - $actual = Config::get('cms.storage.resized.folder') . (!str_starts_with($actual, '/') ? '/' : '') . $actual; - $disk = 'resized'; - } - - if (!ImageResizer::getDefaultDisk()->exists($actual)) { - $actual = $path; - $disk = 'media'; - } - - return [ - $disk === 'resized' - ? $img - : MediaLibrary::validatePath($actual), - MediaLibrary::validatePath($path), - $disk - ]; - } - /** * Resize image AJAX handler * @return array @@ -1477,7 +1432,7 @@ protected function getCropEditImageUrlAndSize($path, $params = null) return [ 'url' => $url, - 'dimensions' => str_starts_with($url, '/') + 'dimensions' => Str::startsWith($url, '/') ? getimagesize(base_path($url)) : getimagesize($url) ]; @@ -1486,16 +1441,41 @@ protected function getCropEditImageUrlAndSize($path, $params = null) /** * Process the provided path and add a suffix of _$int to prevent conflicts * with existing paths + * @TODO: Consider moving this into the File helper and accepting a $disk instance */ - protected function deduplicatePath(string $path): string + protected function deduplicatePath(string $path, string $suffix = null): string { $parts = pathinfo($path); $i = 1; + // Path generation adds a DIRECTORY_SEPARATOR between the dirname and + // the filename so ensure that the dirname doesn't already end with one + $parts['dirname'] = rtrim($parts['dirname'], DIRECTORY_SEPARATOR); + + // Apply the requested suffix to the path + if (!empty($suffix)) { + $parts['filename'] = preg_replace( + // Remove the suffix if it's already there before re-adding it + '/' . preg_quote($suffix, '/') . '(_\d)?/', + '', + $parts['filename'] + ) . $suffix; + + // Regenerate the path so that it can be checked for existance + $path = sprintf( + '%s%s%s.%s', + $parts['dirname'], + DIRECTORY_SEPARATOR, + $parts['filename'], + $parts['extension'] + ); + } + while (MediaLibrary::instance()->exists($path)) { $path = sprintf( - '%s/%s_%d.%s', + '%s%s%s_%d.%s', $parts['dirname'], + DIRECTORY_SEPARATOR, $parts['filename'], $i++, $parts['extension'] diff --git a/modules/system/classes/ImageResizer.php b/modules/system/classes/ImageResizer.php index 182b2097d5..4112fe1bc4 100644 --- a/modules/system/classes/ImageResizer.php +++ b/modules/system/classes/ImageResizer.php @@ -131,10 +131,8 @@ public static function processImage( /** * Get the default options for the resizer - * - * @return array */ - public function getDefaultOptions() + public function getDefaultOptions(): array { // Default options for the built in resizing processor $defaultOptions = [ @@ -164,10 +162,8 @@ public function getDefaultOptions() /** * Get the available sources for processing image resize requests from - * - * @return array */ - public static function getAvailableSources() + public static function getAvailableSources(): array { if (!empty(static::$availableSources)) { return static::$availableSources; @@ -228,10 +224,8 @@ public static function getAvailableSources() /** * Flushes the local sources cache. - * - * @return void */ - public static function flushAvailableSources() + public static function flushAvailableSources(): void { if (empty(static::$availableSources)) { return; @@ -242,10 +236,8 @@ public static function flushAvailableSources() /** * Get the current config - * - * @return array */ - public function getConfig() + public function getConfig(): array { $disk = $this->image['disk']; @@ -295,7 +287,7 @@ public function getConfig() /** * Process the resize request */ - public function resize() + public function resize(): void { if ($this->isResized()) { return; @@ -376,7 +368,7 @@ public function resize() /** * Process the crop request */ - public function crop() + public function crop(): void { if ($this->isResized()) { return; @@ -460,9 +452,9 @@ public function crop() } /** - * Define the internal working path, override this method to define. + * Get the internal temporary drirectory and ensure it exists */ - public function getTempPath() + public function getTempPath(): string { $path = temp_path() . '/resizer'; @@ -477,9 +469,8 @@ public function getTempPath() * Stores the current source image in the temp directory and returns the path to it * * @param string $path The path to suffix the temp directory path with, defaults to $identifier.$ext - * @return string $tempPath */ - protected function getLocalTempPath($path = null) + protected function getLocalTempPath($path = null): string { if (!is_null($path) && is_string($path)) { $tempPath = $this->getTempPath() . '/' . $path; @@ -497,15 +488,13 @@ protected function getLocalTempPath($path = null) /** * Returns the file extension. */ - public function getExtension() + public function getExtension(): string { return FileHelper::extension($this->image['path']); } /** * Get the contents of the image file to be resized - * - * @return string */ public function getSourceFileContents() { @@ -514,10 +503,8 @@ public function getSourceFileContents() /** * Gets the current fileModel associated with the source image if one exists - * - * @return FileModel|null */ - public function getFileModel() + public function getFileModel(): ?FileModel { if ($this->fileModel) { return $this->fileModel; @@ -534,11 +521,17 @@ public function getFileModel() return $this->fileModel; } - public static function getDefaultDisk() + /** + * Get the default disk used to store processed images + */ + public static function getDefaultDisk(): \Illuminate\Contracts\Filesystem\Filesystem { return Storage::disk(Config::get('cms.storage.resized.disk', 'local')); } + /** + * Get the disk instance for image that is currently being processed + */ public function getDisk(): \Illuminate\Contracts\Filesystem\Filesystem { return ($this->image['source'] === 'filemodel' && $fileModel = $this->getFileModel()) @@ -551,7 +544,7 @@ public function getDisk(): \Illuminate\Contracts\Filesystem\Filesystem * * @return array [FilesystemAdapter $disk, (string) $path] */ - protected function getTargetDetails() + protected function getTargetDetails(): array { if ($this->image['source'] === 'filemodel' && $fileModel = $this->getFileModel()) { return [ @@ -568,11 +561,8 @@ protected function getTargetDetails() /** * Get the reference to the resized image if the requested resize exists - * - * @param string $identifier The Resizer Identifier that references the source image and desired resizing configuration - * @return bool|string */ - public function isResized() + public function isResized(): bool { // Get the details for the target image list($disk, $path) = $this->getTargetDetails(); @@ -584,7 +574,7 @@ public function isResized() /** * Get the path of the resized image */ - public function getPathToResizedImage() + public function getPathToResizedImage(): string { // Generate the unique file identifier for the resized image $fileIdentifier = hash_hmac('sha1', serialize($this->getConfig()), Crypt::getKey()); @@ -602,10 +592,8 @@ public function getPathToResizedImage() /** * Gets the current useful URL to the resized image * (resizer if not resized, resized image directly if resized) - * - * @return string */ - public function getUrl() + public function getUrl(): string { if ($this->isResized()) { return $this->getResizedUrl(); @@ -616,10 +604,8 @@ public function getUrl() /** * Get the URL to the system resizer route for this instance's configuration - * - * @return string $url */ - public function getResizerUrl() + public function getResizerUrl(): string { // Slashes in URL params have to be double encoded to survive Laravel's router // @see https://github.com/octobercms/october/issues/3592#issuecomment-671017380 @@ -642,10 +628,8 @@ public function getResizerUrl() /** * Get the URL to the resized image - * - * @return string */ - public function getResizedUrl() + public function getResizedUrl(): string { $url = ''; @@ -681,7 +665,7 @@ public function getResizedUrl() * @return array Array containing the disk, path, source, and fileModel if applicable * ['disk' => FilesystemAdapter, 'path' => string, 'source' => string, 'fileModel' => FileModel|void] */ - public static function normalizeImage($image) + public static function normalizeImage($image): array { $disk = null; $path = null; @@ -817,11 +801,8 @@ public static function normalizeImage($image) * * NOTE: Can't use Winter\Storm\FileSystem\PathResolver because it prepends * the current working directory to relative paths - * - * @param string $path - * @return string */ - protected static function normalizePath($path) + protected static function normalizePath(string $path): string { return str_replace('\\', '/', $path); } @@ -832,7 +813,7 @@ protected static function normalizePath($path) * @param string $id * @return bool */ - public static function isValidIdentifier($id) + public static function isValidIdentifier($id): bool { return is_string($id) && ctype_alnum($id) && strlen($id) === 40; } @@ -842,7 +823,7 @@ public static function isValidIdentifier($id) * * @return string 40 character string used as a unique reference to the provided configuration */ - public function getIdentifier() + public function getIdentifier(): string { if ($this->identifier) { return $this->identifier; @@ -855,7 +836,7 @@ public function getIdentifier() /** * Stores the resizer configuration if the resizing hasn't been completed yet */ - public function storeConfig() + public function storeConfig(): void { // If the image hasn't been resized yet, then store the config data for the resizer to use if (!$this->isResized()) { @@ -868,9 +849,8 @@ public function storeConfig() * * @param string $identifier The 40 character cache identifier for the desired resizer configuration * @throws SystemException If the identifier is unable to be loaded - * @return static */ - public static function fromIdentifier(string $identifier) + public static function fromIdentifier(string $identifier): self { $cacheKey = static::CACHE_PREFIX . $identifier; @@ -896,11 +876,9 @@ public static function fromIdentifier(string $identifier) /** * Check the provided encoded URL to verify its signature and return the decoded URL * - * @param string $identifier - * @param string $encodedUrl * @return string|null Returns null if the provided value was invalid */ - public static function getValidResizedUrl($identifier, $encodedUrl) + public static function getValidResizedUrl(string $identifier, string $encodedUrl): ?string { // Slashes in URL params have to be double encoded to survive Laravel's router // @see https://github.com/octobercms/october/issues/3592#issuecomment-671017380 @@ -926,9 +904,8 @@ public static function getValidResizedUrl($identifier, $encodedUrl) * @param integer|string|bool|null $height Desired height of the resized image * @param array|null $options Array of options to pass to the resizer * @throws Exception If the provided image was unable to be processed - * @return string */ - public static function filterGetUrl($image, $width = null, $height = null, $options = []) + public static function filterGetUrl($image, $width = null, $height = null, $options = []): string { // Attempt to process the provided image try { @@ -956,9 +933,8 @@ public static function filterGetUrl($image, $width = null, $height = null, $opti * instance of Winter\Storm\Database\Attach\File, * string containing URL or path accessible to the application's filesystem manager * @throws SystemException If the provided input was unable to be processed - * @return array ['width' => int, 'height' => int] */ - public static function filterGetDimensions($image) + public static function filterGetDimensions($image): array { $resizer = new static($image);