feat: add support for sharp resize options (#2844)

* feat(ImageResize): add support for resize options

* fix(ImageUpload): reuse name for accidental duplicate

* fix(ImageResize): e2e tests for added media size

* chore: simplify fileExists method

* fix: typo

* feat(ImageResize): update name to be more transparent

* fix: use fileExists in file removal

* improve names, comments and clarity of needsResize function

* fix: jsDoc params

* fix: incorrect needsResize condition and add failing test case

* chore: improve comment

* fix: merge conflict error

---------

Co-authored-by: Alessio Gravili <70709113+AlessioGr@users.noreply.github.com>

src/uploads/deleteAssociatedFiles.ts

@@ -1,11 +1,11 @@
 import path from 'path';
 import fs from 'fs';
 import type { TFunction } from 'i18next';
-import fileExists from './fileExists';
 import { ErrorDeletingFile } from '../errors';
 import type { FileData, FileToSave } from './types';
 import type { SanitizedConfig } from '../config/types';
 import type { SanitizedCollectionConfig } from '../collections/config/types';
+import fileExists from './fileExists';
 
 type Args = {
   config: SanitizedConfig
@@ -31,26 +31,31 @@ export const deleteAssociatedFiles: (args: Args) => Promise<void> = async ({
 
     const fileToDelete = `${staticPath}/${doc.filename}`;
 
-    if (await fileExists(fileToDelete)) {
-      fs.unlink(fileToDelete, (err) => {
-        if (err) {
-          throw new ErrorDeletingFile(t);
-        }
-      });
+    try {
+      if (await fileExists(fileToDelete)) {
+        fs.unlinkSync(fileToDelete);
+      }
+    } catch (err) {
+      throw new ErrorDeletingFile(t);
     }
 
     if (doc.sizes) {
-      Object.values(doc.sizes)
-        .forEach(async (size: FileData) => {
-          const sizeToDelete = `${staticPath}/${size.filename}`;
+      const sizes: FileData[] = Object.values(doc.sizes);
+      // Since forEach will not wait until unlink is finished it could
+      // happen that two operations will try to delete the same file.
+      // To avoid this it is recommended to use "sync" instead
+      // eslint-disable-next-line no-restricted-syntax
+      for (const size of sizes) {
+        const sizeToDelete = `${staticPath}/${size.filename}`;
+        try {
+          // eslint-disable-next-line no-await-in-loop
           if (await fileExists(sizeToDelete)) {
-            fs.unlink(sizeToDelete, (err) => {
-              if (err) {
-                throw new ErrorDeletingFile(t);
-              }
-            });
+            fs.unlinkSync(sizeToDelete);
           }
-        });
+        } catch (err) {
+          throw new ErrorDeletingFile(t);
+        }
+      }
     }
   }
 };

src/uploads/fileExists.ts

@@ -1,11 +1,8 @@
 import fs from 'fs';
-import { promisify } from 'util';
-
-const stat = promisify(fs.stat);
 
 const fileExists = async (filename: string): Promise<boolean> => {
   try {
-    await stat(filename);
+    await fs.promises.stat(filename);
 
     return true;
   } catch (err) {

src/uploads/generateFileData.ts

@@ -9,7 +9,7 @@ import { FileUploadError, MissingFile } from '../errors';
 import { PayloadRequest } from '../express/types';
 import getImageSize from './getImageSize';
 import getSafeFileName from './getSafeFilename';
-import resizeAndSave from './imageResizer';
+import resizeAndTransformImageSizes from './imageResizer';
 import { FileData, FileToSave, ProbedImageSize } from './types';
 import canResizeImage from './canResizeImage';
 import isImage from './isImage';
@@ -155,7 +155,7 @@ export const generateFileData = async <T>({
     if (Array.isArray(imageSizes) && fileSupportsResize) {
       req.payloadUploadSizes = {};
 
-      const { sizeData, sizesToSave } = await resizeAndSave({
+      const { sizeData, sizesToSave } = await resizeAndTransformImageSizes({
         req,
         file,
         dimensions,

src/uploads/imageResizer.ts

@@ -6,128 +6,229 @@ import sharp from 'sharp';
 import { SanitizedCollectionConfig } from '../collections/config/types';
 import { PayloadRequest } from '../express/types';
 import fileExists from './fileExists';
-import { FileSizes, FileToSave, ImageSize } from './types';
+import { FileSize, FileSizes, FileToSave, ImageSize, ProbedImageSize } from './types';
 
-type Dimensions = { width?: number, height?: number }
+type ResizeArgs = {
+  req: PayloadRequest;
+  file: UploadedFile;
+  dimensions: ProbedImageSize;
+  staticPath: string;
+  config: SanitizedCollectionConfig;
+  savedFilename: string;
+  mimeType: string;
+};
 
-type Args = {
-  req: PayloadRequest
-  file: UploadedFile
-  dimensions: Dimensions
-  staticPath: string
-  config: SanitizedCollectionConfig
-  savedFilename: string
-  mimeType: string
-}
+/** Result from resizing and transforming the requested image sizes */
+type ImageSizesResult = {
+  sizeData: FileSizes;
+  sizesToSave: FileToSave[];
+};
 
-type OutputImage = {
-  name: string,
-  extension: string,
-  width: number,
-  height: number
-}
+type SanitizedImageData = {
+  name: string;
+  ext: string;
+};
 
-type Result = Promise<{
-  sizeData: FileSizes
-  sizesToSave: FileToSave[]
-}>
-
-function getOutputImage(sourceImage: string, size: ImageSize): OutputImage {
+/**
+ * Sanitize the image name and extract the extension from the source image
+ *
+ * @param sourceImage - the source image
+ * @returns the sanitized name and extension
+ */
+const getSanitizedImageData = (sourceImage: string): SanitizedImageData => {
   const extension = sourceImage.split('.').pop();
   const name = sanitize(sourceImage.substring(0, sourceImage.lastIndexOf('.')) || sourceImage);
+  return { name, ext: extension! };
+};
 
-  return {
-    name,
-    extension,
-    width: size.width,
-    height: size.height,
-  };
-}
+/**
+ * Create a new image name based on the output image name, the dimensions and
+ * the extension.
+ *
+ * Ignore the fact that duplicate names could happen if the there is one
+ * size with `width AND height` and one with only `height OR width`. Because
+ * space is expensive, we will reuse the same image for both sizes.
+ *
+ * @param outputImageName - the sanitized image name
+ * @param bufferInfo - the buffer info
+ * @param extension - the extension to use
+ * @returns the new image name that is not taken
+ */
+const createImageName = (
+  outputImageName: string,
+  { width, height }: sharp.OutputInfo,
+  extension: string,
+) => `${outputImageName}-${width}x${height}.${extension}`;
 
-export default async function resizeAndSave({
+/**
+ * Create the result object for the image resize operation based on the
+ * provided parameters. If the name is not provided, an empty result object
+ * is returned.
+ *
+ * @param name - the name of the image
+ * @param filename - the filename of the image
+ * @param width - the width of the image
+ * @param height - the height of the image
+ * @param filesize - the filesize of the image
+ * @param mimeType - the mime type of the image
+ * @param sizesToSave - the sizes to save
+ * @returns the result object
+ */
+const createResult = (
+  name: string,
+  filename: FileSize['filename'] = null,
+  width: FileSize['width'] = null,
+  height: FileSize['height'] = null,
+  filesize: FileSize['filesize'] = null,
+  mimeType: FileSize['mimeType'] = null,
+  sizesToSave: FileToSave[] = [],
+): ImageSizesResult => ({
+  sizesToSave,
+  sizeData: {
+    [name]: {
+      width,
+      height,
+      filename,
+      filesize,
+      mimeType,
+    },
+  },
+});
+
+/**
+ * Check if the image needs to be resized according to the requested dimensions
+ * and the original image size. If the resize options withoutEnlargement or withoutReduction are provided,
+ * the image will be resized regardless of the requested dimensions, given that the
+ * width or height to be resized is provided.
+ *
+ * @param resizeConfig - object containing the requested dimensions and resize options
+ * @param original - the original image size
+ * @returns true if the image needs to be resized, false otherwise
+ */
+const needsResize = (
+  { width: desiredWidth, height: desiredHeigth, withoutEnlargement, withoutReduction }: ImageSize,
+  original: ProbedImageSize,
+): boolean => {
+  // allow enlargement or prevent reduction (our default is to prevent
+  // enlargement and allow reduction)
+  if (withoutEnlargement !== undefined || withoutReduction !== undefined) {
+    return true; // needs resize
+  }
+
+  const isWidthOrHeightNotDefined = !desiredHeigth || !desiredWidth;
+  if (isWidthOrHeightNotDefined) {
+    // If with and height are not defined, it means there is a format conversion
+    // and the image needs to be "resized" (transformed).
+    return true; // needs resize
+  }
+
+  const hasInsufficientWidth = original.width < desiredWidth;
+  const hasInsufficientHeight = original.height < desiredHeigth;
+  if (hasInsufficientWidth && hasInsufficientHeight) {
+    // doesn't need resize - prevent enlargement. This should only happen if both width and height are insufficient.
+    // if only one dimension is insufficient and the other is sufficient, resizing needs to happen, as the image
+    // should be resized to the sufficient dimension.
+    return false;
+  }
+
+  return true; // needs resize
+};
+
+/**
+ * For the provided image sizes, handle the resizing and the transforms
+ * (format, trim, etc.) of each requested image size and return the result object.
+ * This only handles the image sizes. The transforms of the original image
+ * are handled in {@link ./generateFileData.ts}.
+ *
+ * The image will be resized according to the provided
+ * resize config. If no image sizes are requested, the resolved data will be empty.
+ * For every image that dos not need to be resized, an result object with `null`
+ * parameters will be returned.
+ *
+ * @param resizeConfig - the resize config
+ * @returns the result of the resize operation(s)
+ */
+export default async function resizeAndTransformImageSizes({
   req,
   file,
   dimensions,
   staticPath,
   config,
   savedFilename,
-}: Args): Promise<Result> {
+  mimeType,
+}: ResizeArgs): Promise<ImageSizesResult> {
   const { imageSizes } = config.upload;
-  const sizesToSave: FileToSave[] = [];
-  const sizeData = {};
+
+  // Noting to resize here so return as early as possible
+  if (!imageSizes) return { sizeData: {}, sizesToSave: [] };
 
   const sharpBase = sharp(file.tempFilePath || file.data).rotate(); // pass rotate() to auto-rotate based on EXIF data. https://github.com/payloadcms/payload/pull/3081
 
-  const promises = imageSizes
-    .map(async (desiredSize) => {
-      if (!needsResize(desiredSize, dimensions)) {
-        sizeData[desiredSize.name] = {
-          url: null,
-          width: null,
-          height: null,
-          filename: null,
-          filesize: null,
-          mimeType: null,
-        };
-        return;
-      }
-      let resized = sharpBase.clone().resize(desiredSize);
-
-      if (desiredSize.formatOptions) {
-        resized = resized.toFormat(desiredSize.formatOptions.format, desiredSize.formatOptions.options);
+  const results: ImageSizesResult[] = await Promise.all(
+    imageSizes.map(async (imageResizeConfig): Promise<ImageSizesResult> => {
+      // This checks if a resize should happen. If not, the resized image will be
+      // skipped COMPLETELY and thus will not be included in the resulting images.
+      // All further format/trim options will thus be skipped as well.
+      if (!needsResize(imageResizeConfig, dimensions)) {
+        return createResult(imageResizeConfig.name);
       }
 
-      if (desiredSize.trimOptions) {
-        resized = resized.trim(desiredSize.trimOptions);
+      let resized = sharpBase.clone().resize(imageResizeConfig);
+
+      if (imageResizeConfig.formatOptions) {
+        resized = resized.toFormat(
+          imageResizeConfig.formatOptions.format,
+          imageResizeConfig.formatOptions.options,
+        );
       }
 
-      const bufferObject = await resized.toBuffer({
+      if (imageResizeConfig.trimOptions) {
+        resized = resized.trim(imageResizeConfig.trimOptions);
+      }
+
+      const { data: bufferData, info: bufferInfo } = await resized.toBuffer({
         resolveWithObject: true,
       });
 
-      req.payloadUploadSizes[desiredSize.name] = bufferObject.data;
+      const sanitizedImage = getSanitizedImageData(savedFilename);
+
+      if (req.payloadUploadSizes) {
+        req.payloadUploadSizes[imageResizeConfig.name] = bufferData;
+      }
+
+      const mimeInfo = await fromBuffer(bufferData);
+
+      const imageNameWithDimensions = createImageName(
+        sanitizedImage.name,
+        bufferInfo,
+        mimeInfo?.ext || sanitizedImage.ext,
+      );
 
-      const mimeType = (await fromBuffer(bufferObject.data));
-      const outputImage = getOutputImage(savedFilename, desiredSize);
-      const imageNameWithDimensions = createImageName(outputImage, bufferObject, mimeType.ext);
       const imagePath = `${staticPath}/${imageNameWithDimensions}`;
-      const fileAlreadyExists = await fileExists(imagePath);
 
-      if (fileAlreadyExists) {
+      if (await fileExists(imagePath)) {
         fs.unlinkSync(imagePath);
       }
 
-      sizesToSave.push({
-        path: imagePath,
-        buffer: bufferObject.data,
-      });
+      const { width, height, size } = bufferInfo;
+      return createResult(
+        imageResizeConfig.name,
+        imageNameWithDimensions,
+        width,
+        height,
+        size,
+        mimeInfo?.mime || mimeType,
+        [{ path: imagePath, buffer: bufferData }],
+      );
+    }),
+  );
 
-      sizeData[desiredSize.name] = {
-        width: bufferObject.info.width,
-        height: bufferObject.info.height,
-        filename: imageNameWithDimensions,
-        filesize: bufferObject.info.size,
-        mimeType: mimeType.mime,
-      };
-    });
-
-  await Promise.all(promises);
-
-  return {
-    sizeData,
-    sizesToSave,
-  };
-}
-function createImageName(
-  outputImage: OutputImage,
-  bufferObject: { data: Buffer; info: sharp.OutputInfo },
-  extension: string,
-): string {
-  return `${outputImage.name}-${bufferObject.info.width}x${bufferObject.info.height}.${extension}`;
-}
-
-function needsResize(desiredSize: ImageSize, dimensions: Dimensions): boolean {
-  return (typeof desiredSize.width === 'number' && desiredSize.width <= dimensions.width)
-    || (typeof desiredSize.height === 'number' && desiredSize.height <= dimensions.height)
-    || (!desiredSize.height && !desiredSize.width);
+  return results.reduce(
+    (acc, result) => {
+      Object.assign(acc.sizeData, result.sizeData);
+      acc.sizesToSave.push(...result.sizesToSave);
+      return acc;
+    },
+    { sizeData: {}, sizesToSave: [] },
+  );
 }

src/uploads/types.ts

@@ -4,12 +4,12 @@ import serveStatic from 'serve-static';
 import { Sharp, ResizeOptions } from 'sharp';
 
 export type FileSize = {
-  filename: string;
-  filesize: number;
-  mimeType: string;
-  width: number;
-  height: number;
-}
+  filename: string | null;
+  filesize: number | null;
+  mimeType: string | null;
+  width: number | null;
+  height: number | null;
+};
 
 export type FileSizes = {
   [size: string]: FileSize