Merge pull request #10528 from growilabs/imprv/refactor-attachment-service

imprv: Refactor AttachmentService

apps/app/src/server/service/attachment.js → apps/app/src/server/service/attachment.ts

@@ -1,5 +1,8 @@
+import type { IAttachment } from '@growi/core/dist/interfaces';
+
 import loggerFactory from '~/utils/logger';
 
+import type Crowi from '../crowi';
 import { AttachmentType } from '../interfaces/attachment';
 import { Attachment } from '../models/attachment';
 
@@ -16,22 +19,23 @@ const createReadStream = (filePath) => {
   });
 };
 
+type AttachHandler = (pageId: string | null, attachment: IAttachment, file: Express.Multer.File) => Promise<void>;
+
+type DetachHandler = (attachmentId: string) => Promise<void>;
+
+
 /**
  * the service class for Attachment and file-uploader
  */
 class AttachmentService {
 
-  /** @type {Array<(pageId: string, attachment: Attachment, file: Express.Multer.File) => Promise<void>>} */
-  attachHandlers = [];
+  attachHandlers: AttachHandler[] = [];
 
-  /** @type {Array<(attachmentId: string) => Promise<void>>} */
-  detachHandlers = [];
+  detachHandlers: DetachHandler[] = [];
 
-  /** @type {import('~/server/crowi').default} Crowi instance */
-  crowi;
+  crowi: Crowi;
 
-  /** @param {import('~/server/crowi').default} crowi Crowi instance */
-  constructor(crowi) {
+  constructor(crowi: Crowi) {
     this.crowi = crowi;
   }
 
@@ -101,6 +105,10 @@ class AttachmentService {
     const { fileUploadService } = this.crowi;
     const attachment = await Attachment.findById(attachmentId);
 
+    if (attachment == null) {
+      throw new Error(`Attachment not found: ${attachmentId}`);
+    }
+
     await fileUploadService.deleteFile(attachment);
     await attachment.remove();
 

apps/app/src/server/service/file-uploader/aws/index.ts

@@ -166,8 +166,50 @@ class AwsFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override deleteFiles() {
-    throw new Error('Method not implemented.');
+  override async deleteFile(attachment: IAttachmentDocument): Promise<void> {
+    const filePath = getFilePathOnStorage(attachment);
+    return this.deleteFileByFilePath(filePath);
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override async deleteFiles(attachments: IAttachmentDocument[]): Promise<void> {
+    if (!this.getIsUploadable()) {
+      throw new Error('AWS is not configured.');
+    }
+    const s3 = S3Factory();
+
+    const filePaths = attachments.map((attachment) => {
+      return { Key: getFilePathOnStorage(attachment) };
+    });
+
+    const totalParams = {
+      Bucket: getS3Bucket(),
+      Delete: { Objects: filePaths },
+    };
+    await s3.send(new DeleteObjectsCommand(totalParams));
+  }
+
+  private async deleteFileByFilePath(filePath: string): Promise<void> {
+    if (!this.getIsUploadable()) {
+      throw new Error('AWS is not configured.');
+    }
+    const s3 = S3Factory();
+
+    const params = {
+      Bucket: getS3Bucket(),
+      Key: filePath,
+    };
+
+    // check file exists
+    const isExists = await isFileExists(s3, params);
+    if (!isExists) {
+      logger.warn(`Any object that relate to the Attachment (${filePath}) does not exist in AWS S3`);
+      return;
+    }
+
+    await s3.send(new DeleteObjectCommand(params));
   }
 
   /**
@@ -345,49 +387,6 @@ module.exports = (crowi: Crowi) => {
       && configManager.getConfig('aws:s3Bucket') != null;
   };
 
-  (lib as any).deleteFile = async function(attachment) {
-    const filePath = getFilePathOnStorage(attachment);
-    return (lib as any).deleteFileByFilePath(filePath);
-  };
-
-  (lib as any).deleteFiles = async function(attachments) {
-    if (!lib.getIsUploadable()) {
-      throw new Error('AWS is not configured.');
-    }
-    const s3 = S3Factory();
-
-    const filePaths = attachments.map((attachment) => {
-      return { Key: getFilePathOnStorage(attachment) };
-    });
-
-    const totalParams = {
-      Bucket: getS3Bucket(),
-      Delete: { Objects: filePaths },
-    };
-    return s3.send(new DeleteObjectsCommand(totalParams));
-  };
-
-  (lib as any).deleteFileByFilePath = async function(filePath) {
-    if (!lib.getIsUploadable()) {
-      throw new Error('AWS is not configured.');
-    }
-    const s3 = S3Factory();
-
-    const params = {
-      Bucket: getS3Bucket(),
-      Key: filePath,
-    };
-
-    // check file exists
-    const isExists = await isFileExists(s3, params);
-    if (!isExists) {
-      logger.warn(`Any object that relate to the Attachment (${filePath}) does not exist in AWS S3`);
-      return;
-    }
-
-    return s3.send(new DeleteObjectCommand(params));
-  };
-
   lib.saveFile = async function({ filePath, contentType, data }) {
     const s3 = S3Factory();
 
@@ -400,12 +399,6 @@ module.exports = (crowi: Crowi) => {
     }));
   };
 
-  (lib as any).checkLimit = async function(uploadFileSize) {
-    const maxFileSize = configManager.getConfig('app:maxFileSize');
-    const totalLimit = configManager.getConfig('app:fileUploadTotalLimit');
-    return lib.doCheckLimit(uploadFileSize, maxFileSize, totalLimit);
-  };
-
   /**
    * List files in storage
    */

apps/app/src/server/service/file-uploader/azure.ts

@@ -161,8 +161,27 @@ class AzureFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override deleteFiles() {
-    throw new Error('Method not implemented.');
+  override async deleteFile(attachment: IAttachmentDocument): Promise<void> {
+    const filePath = getFilePathOnStorage(attachment);
+    const containerClient = await getContainerClient();
+    const blockBlobClient = await containerClient.getBlockBlobClient(filePath);
+    const options: BlobDeleteOptions = { deleteSnapshots: 'include' };
+    const blobDeleteIfExistsResponse: BlobDeleteIfExistsResponse = await blockBlobClient.deleteIfExists(options);
+    if (!blobDeleteIfExistsResponse.errorCode) {
+      logger.info(`deleted blob ${filePath}`);
+    }
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override async deleteFiles(attachments: IAttachmentDocument[]): Promise<void> {
+    if (!this.getIsUploadable()) {
+      throw new Error('Azure is not configured.');
+    }
+    for await (const attachment of attachments) {
+      await this.deleteFile(attachment);
+    }
   }
 
   /**
@@ -312,26 +331,6 @@ module.exports = (crowi: Crowi) => {
       && configManager.getConfig('azure:storageContainerName') != null;
   };
 
-  (lib as any).deleteFile = async function(attachment) {
-    const filePath = getFilePathOnStorage(attachment);
-    const containerClient = await getContainerClient();
-    const blockBlobClient = await containerClient.getBlockBlobClient(filePath);
-    const options: BlobDeleteOptions = { deleteSnapshots: 'include' };
-    const blobDeleteIfExistsResponse: BlobDeleteIfExistsResponse = await blockBlobClient.deleteIfExists(options);
-    if (!blobDeleteIfExistsResponse.errorCode) {
-      logger.info(`deleted blob ${filePath}`);
-    }
-  };
-
-  (lib as any).deleteFiles = async function(attachments) {
-    if (!lib.getIsUploadable()) {
-      throw new Error('Azure is not configured.');
-    }
-    for await (const attachment of attachments) {
-      (lib as any).deleteFile(attachment);
-    }
-  };
-
   lib.saveFile = async function({ filePath, contentType, data }) {
     const containerClient = await getContainerClient();
     const blockBlobClient: BlockBlobClient = containerClient.getBlockBlobClient(filePath);
@@ -345,12 +344,6 @@ module.exports = (crowi: Crowi) => {
     return;
   };
 
-  (lib as any).checkLimit = async function(uploadFileSize) {
-    const maxFileSize = configManager.getConfig('app:maxFileSize');
-    const gcsTotalLimit = configManager.getConfig('app:fileUploadTotalLimit');
-    return lib.doCheckLimit(uploadFileSize, maxFileSize, gcsTotalLimit);
-  };
-
   (lib as any).listFiles = async function() {
     if (!lib.getIsReadable()) {
       throw new Error('Azure is not configured.');

apps/app/src/server/service/file-uploader/file-uploader.ts

@@ -1,6 +1,7 @@
 import type { Readable } from 'stream';
 
 import type { Response } from 'express';
+import type { HydratedDocument } from 'mongoose';
 import { v4 as uuidv4 } from 'uuid';
 
 import type { ICheckLimitResult } from '~/interfaces/attachment';
@@ -35,10 +36,11 @@ export interface FileUploader {
   getFileUploadEnabled(): boolean,
   listFiles(): any,
   saveFile(param: SaveFileParam): Promise<any>,
-  deleteFiles(): void,
+  deleteFile(attachment: HydratedDocument<IAttachmentDocument>): void,
+  deleteFiles(attachments: HydratedDocument<IAttachmentDocument>[]): void,
   getFileUploadTotalLimit(): number,
   getTotalFileSize(): Promise<number>,
-  doCheckLimit(uploadFileSize: number, maxFileSize: number, totalLimit: number): Promise<ICheckLimitResult>,
+  checkLimit(uploadFileSize: number): Promise<ICheckLimitResult>,
   determineResponseMode(): ResponseMode,
   uploadAttachment(readable: Readable, attachment: IAttachmentDocument): Promise<void>,
   respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): void,
@@ -103,20 +105,16 @@ export abstract class AbstractFileUploader implements FileUploader {
 
   abstract saveFile(param: SaveFileParam);
 
-  abstract deleteFiles();
+  abstract deleteFile(attachment: HydratedDocument<IAttachmentDocument>): void;
+
+  abstract deleteFiles(attachments: HydratedDocument<IAttachmentDocument>[]): void;
 
   /**
    * Returns file upload total limit in bytes.
-   * Reference to previous implementation is
-   * {@link https://github.com/growilabs/growi/blob/798e44f14ad01544c1d75ba83d4dfb321a94aa0b/src/server/service/file-uploader/gridfs.js#L86-L88}
    * @returns file upload total limit in bytes
    */
-  getFileUploadTotalLimit() {
-    const fileUploadTotalLimit = configManager.getConfig('app:fileUploadType') === 'mongodb'
-      // Use app:fileUploadTotalLimit if gridfs:totalLimit is null (default for gridfs:totalLimit is null)
-      ? configManager.getConfig('app:fileUploadTotalLimit')
-      : configManager.getConfig('app:fileUploadTotalLimit');
-    return fileUploadTotalLimit;
+  getFileUploadTotalLimit(): number {
+    return configManager.getConfig('app:fileUploadTotalLimit');
   }
 
   /**
@@ -134,11 +132,20 @@ export abstract class AbstractFileUploader implements FileUploader {
     return res.length === 0 ? 0 : res[0].total;
   }
 
+  /**
+   * check the file size limit
+   */
+  checkLimit(uploadFileSize: number): Promise<ICheckLimitResult> {
+    const maxFileSize = configManager.getConfig('app:maxFileSize');
+    const totalLimit = this.getFileUploadTotalLimit();
+    return this.doCheckLimit(uploadFileSize, maxFileSize, totalLimit);
+  }
+
   /**
    * Check files size limits for all uploaders
    *
    */
-  async doCheckLimit(uploadFileSize: number, maxFileSize: number, totalLimit: number): Promise<ICheckLimitResult> {
+  protected async doCheckLimit(uploadFileSize: number, maxFileSize: number, totalLimit: number): Promise<ICheckLimitResult> {
     if (uploadFileSize > maxFileSize) {
       return { isUploadable: false, errorMessage: 'File size exceeds the size limit per file' };
     }

apps/app/src/server/service/file-uploader/gcs/index.ts

@@ -105,8 +105,36 @@ class GcsFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override deleteFiles() {
-    throw new Error('Method not implemented.');
+  override async deleteFile(attachment: IAttachmentDocument): Promise<void> {
+    const filePath = getFilePathOnStorage(attachment);
+    return this.deleteFilesByFilePaths([filePath]);
+  }
+
+  /**
+   * @inheritdoc
+   */
+  override async deleteFiles(attachments: IAttachmentDocument[]): Promise<void> {
+    const filePaths = attachments.map((attachment) => {
+      return getFilePathOnStorage(attachment);
+    });
+    return this.deleteFilesByFilePaths(filePaths);
+  }
+
+  private async deleteFilesByFilePaths(filePaths: string[]): Promise<void> {
+    if (!this.getIsUploadable()) {
+      throw new Error('GCS is not configured.');
+    }
+
+    const gcs = getGcsInstance();
+    const myBucket = gcs.bucket(getGcsBucket());
+
+    const files = filePaths.map((filePath) => {
+      return myBucket.file(filePath);
+    });
+
+    files.forEach((file) => {
+      file.delete({ ignoreNotFound: true });
+    });
   }
 
   /**
@@ -263,35 +291,6 @@ module.exports = function(crowi: Crowi) {
       && configManager.getConfig('gcs:bucket') != null;
   };
 
-  (lib as any).deleteFile = function(attachment) {
-    const filePath = getFilePathOnStorage(attachment);
-    return (lib as any).deleteFilesByFilePaths([filePath]);
-  };
-
-  (lib as any).deleteFiles = function(attachments) {
-    const filePaths = attachments.map((attachment) => {
-      return getFilePathOnStorage(attachment);
-    });
-    return (lib as any).deleteFilesByFilePaths(filePaths);
-  };
-
-  (lib as any).deleteFilesByFilePaths = function(filePaths) {
-    if (!lib.getIsUploadable()) {
-      throw new Error('GCS is not configured.');
-    }
-
-    const gcs = getGcsInstance();
-    const myBucket = gcs.bucket(getGcsBucket());
-
-    const files = filePaths.map((filePath) => {
-      return myBucket.file(filePath);
-    });
-
-    files.forEach((file) => {
-      file.delete({ ignoreNotFound: true });
-    });
-  };
-
   lib.saveFile = async function({ filePath, contentType, data }) {
     const gcs = getGcsInstance();
     const myBucket = gcs.bucket(getGcsBucket());
@@ -299,18 +298,6 @@ module.exports = function(crowi: Crowi) {
     return myBucket.file(filePath).save(data, { resumable: false });
   };
 
-  /**
-   * check the file size limit
-   *
-   * In detail, the followings are checked.
-   * - per-file size limit (specified by MAX_FILE_SIZE)
-   */
-  (lib as any).checkLimit = async function(uploadFileSize) {
-    const maxFileSize = configManager.getConfig('app:maxFileSize');
-    const gcsTotalLimit = configManager.getConfig('app:fileUploadTotalLimit');
-    return lib.doCheckLimit(uploadFileSize, maxFileSize, gcsTotalLimit);
-  };
-
   /**
    * List files in storage
    */

apps/app/src/server/service/file-uploader/gridfs.ts

@@ -104,8 +104,48 @@ class GridfsFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override deleteFiles() {
-    throw new Error('Method not implemented.');
+  override async deleteFile(attachment: IAttachmentDocument): Promise<void> {
+    const { attachmentFileModel } = initializeGridFSModels();
+    const filenameValue = attachment.fileName;
+
+    const attachmentFile = await attachmentFileModel.findOne({ filename: filenameValue });
+
+    if (attachmentFile == null) {
+      logger.warn(`Any AttachmentFile that relate to the Attachment (${attachment._id.toString()}) does not exist in GridFS`);
+      return;
+    }
+
+    return attachmentFileModel.promisifiedUnlink({ _id: attachmentFile._id });
+  }
+
+  /**
+   * @inheritdoc
+   *
+   * Bulk delete files since unlink method of mongoose-gridfs does not support bulk operation
+   */
+  override async deleteFiles(attachments: IAttachmentDocument[]): Promise<void> {
+    const { attachmentFileModel, chunkCollection } = initializeGridFSModels();
+
+    const filenameValues = attachments.map((attachment) => {
+      return attachment.fileName;
+    });
+    const fileIdObjects = await attachmentFileModel.find({ filename: { $in: filenameValues } }, { _id: 1 });
+    const idsRelatedFiles = fileIdObjects.map((obj) => { return obj._id });
+
+    await Promise.all([
+      attachmentFileModel.deleteMany({ filename: { $in: filenameValues } }),
+      chunkCollection.deleteMany({ files_id: { $in: idsRelatedFiles } }),
+    ]);
+  }
+
+  /**
+   * @inheritdoc
+   *
+   * Reference to previous implementation is
+   * {@link https://github.com/growilabs/growi/blob/798e44f14ad01544c1d75ba83d4dfb321a94aa0b/src/server/service/file-uploader/gridfs.js#L86-L88}
+   */
+  override getFileUploadTotalLimit() {
+    return configManager.getConfig('gridfs:totalLimit') ?? configManager.getConfig('app:fileUploadTotalLimit');
   }
 
   /**
@@ -158,51 +198,6 @@ module.exports = function(crowi: Crowi) {
     return true;
   };
 
-  (lib as any).deleteFile = async function(attachment) {
-    const { attachmentFileModel } = initializeGridFSModels();
-    const filenameValue = attachment.fileName;
-
-    const attachmentFile = await attachmentFileModel.findOne({ filename: filenameValue });
-
-    if (attachmentFile == null) {
-      logger.warn(`Any AttachmentFile that relate to the Attachment (${attachment._id.toString()}) does not exist in GridFS`);
-      return;
-    }
-
-    return attachmentFileModel.promisifiedUnlink({ _id: attachmentFile._id });
-  };
-
-  /**
-   * Bulk delete files since unlink method of mongoose-gridfs does not support bulk operation
-   */
-  (lib as any).deleteFiles = async function(attachments) {
-    const { attachmentFileModel, chunkCollection } = initializeGridFSModels();
-
-    const filenameValues = attachments.map((attachment) => {
-      return attachment.fileName;
-    });
-    const fileIdObjects = await attachmentFileModel.find({ filename: { $in: filenameValues } }, { _id: 1 });
-    const idsRelatedFiles = fileIdObjects.map((obj) => { return obj._id });
-
-    return Promise.all([
-      attachmentFileModel.deleteMany({ filename: { $in: filenameValues } }),
-      chunkCollection.deleteMany({ files_id: { $in: idsRelatedFiles } }),
-    ]);
-  };
-
-  /**
-   * check the file size limit
-   *
-   * In detail, the followings are checked.
-   * - per-file size limit (specified by MAX_FILE_SIZE)
-   * - mongodb(gridfs) size limit (specified by MONGO_GRIDFS_TOTAL_LIMIT)
-   */
-  (lib as any).checkLimit = async function(uploadFileSize) {
-    const maxFileSize = configManager.getConfig('app:maxFileSize');
-    const totalLimit = lib.getFileUploadTotalLimit();
-    return lib.doCheckLimit(uploadFileSize, maxFileSize, totalLimit);
-  };
-
   lib.saveFile = async function({ filePath, contentType, data }) {
     const { attachmentFileModel } = initializeGridFSModels();
 

apps/app/src/server/service/file-uploader/local.ts

@@ -56,11 +56,34 @@ class LocalFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override deleteFiles() {
-    throw new Error('Method not implemented.');
+  override async deleteFile(attachment: IAttachmentDocument): Promise<void> {
+    const filePath = this.getFilePathOnStorage(attachment);
+    return this.deleteFileByFilePath(filePath);
   }
 
-  deleteFileByFilePath(filePath: string): void {
+  /**
+   * @inheritdoc
+   */
+  override async deleteFiles(attachments: IAttachmentDocument[]): Promise<void> {
+    await Promise.all(attachments.map((attachment) => {
+      return this.deleteFile(attachment);
+    }));
+  }
+
+  private async deleteFileByFilePath(filePath: string): Promise<void> {
+    // check file exists
+    try {
+      fs.statSync(filePath);
+    }
+    catch (err) {
+      logger.warn(`Any AttachmentFile which path is '${filePath}' does not exist in local fs`);
+      return;
+    }
+
+    return fs.unlinkSync(filePath);
+  }
+
+  getFilePathOnStorage(_attachment: IAttachmentDocument): string {
     throw new Error('Method not implemented.');
   }
 
@@ -108,14 +131,14 @@ module.exports = function(crowi: Crowi) {
 
   const basePath = path.posix.join(crowi.publicDir, 'uploads');
 
-  function getFilePathOnStorage(attachment: IAttachmentDocument) {
+  lib.getFilePathOnStorage = function(attachment: IAttachmentDocument) {
     const dirName = (attachment.page != null)
       ? FilePathOnStoragePrefix.attachment
       : FilePathOnStoragePrefix.user;
     const filePath = path.posix.join(basePath, dirName, attachment.fileName);
 
     return filePath;
-  }
+  };
 
   async function readdirRecursively(dirPath) {
     const directories = await fsPromises.readdir(dirPath, { withFileTypes: true });
@@ -131,34 +154,10 @@ module.exports = function(crowi: Crowi) {
     return true;
   };
 
-  (lib as any).deleteFile = async function(attachment) {
-    const filePath = getFilePathOnStorage(attachment);
-    return lib.deleteFileByFilePath(filePath);
-  };
-
-  (lib as any).deleteFiles = async function(attachments) {
-    attachments.map((attachment) => {
-      return (lib as any).deleteFile(attachment);
-    });
-  };
-
-  lib.deleteFileByFilePath = async function(filePath) {
-    // check file exists
-    try {
-      fs.statSync(filePath);
-    }
-    catch (err) {
-      logger.warn(`Any AttachmentFile which path is '${filePath}' does not exist in local fs`);
-      return;
-    }
-
-    return fs.unlinkSync(filePath);
-  };
-
   lib.uploadAttachment = async function(fileStream, attachment) {
     logger.debug(`File uploading: fileName=${attachment.fileName}`);
 
-    const filePath = getFilePathOnStorage(attachment);
+    const filePath = lib.getFilePathOnStorage(attachment);
     const dirpath = path.posix.dirname(filePath);
 
     // mkdir -p
@@ -211,7 +210,7 @@ module.exports = function(crowi: Crowi) {
    * @return {stream.Readable} readable stream
    */
   lib.findDeliveryFile = async function(attachment) {
-    const filePath = getFilePathOnStorage(attachment);
+    const filePath = lib.getFilePathOnStorage(attachment);
 
     // check file exists
     try {
@@ -225,18 +224,6 @@ module.exports = function(crowi: Crowi) {
     return fs.createReadStream(filePath);
   };
 
-  /**
-   * check the file size limit
-   *
-   * In detail, the followings are checked.
-   * - per-file size limit (specified by MAX_FILE_SIZE)
-   */
-  (lib as any).checkLimit = async function(uploadFileSize) {
-    const maxFileSize = configManager.getConfig('app:maxFileSize');
-    const totalLimit = configManager.getConfig('app:fileUploadTotalLimit');
-    return lib.doCheckLimit(uploadFileSize, maxFileSize, totalLimit);
-  };
-
   /**
    * Respond to the HTTP request.
    * @param {Response} res
@@ -244,7 +231,7 @@ module.exports = function(crowi: Crowi) {
    */
   lib.respond = function(res, attachment, opts) {
     // Responce using internal redirect of nginx or Apache.
-    const storagePath = getFilePathOnStorage(attachment);
+    const storagePath = lib.getFilePathOnStorage(attachment);
     const relativePath = path.relative(crowi.publicDir, storagePath);
     const internalPathRoot = configManager.getConfig('fileUpload:local:internalRedirectPath');
     const internalPath = urljoin(internalPathRoot, relativePath);