refactor: cut thumbnail not block record operations (#1024)

* refactor: cut thumbnail not block record operations * fix: add event and fix e2e * fix: e2e * fix: local unit tests
teableio · Oct 25, 2024 · 706b1c8 · 706b1c8
1 parent c398cf4
commit 706b1c8
Show file tree

Hide file tree

Showing 16 changed files with 136 additions and 99 deletions.
diff --git a/apps/nestjs-backend/src/event-emitter/events/event.enum.ts b/apps/nestjs-backend/src/event-emitter/events/event.enum.ts
@@ -61,4 +61,6 @@ export enum Events {
 
   WORKFLOW_ACTIVATE = 'workflow.activate',
   WORKFLOW_DEACTIVATE = 'workflow.deactivate',
+
+  CROP_IMAGE = 'crop.image',
 }
diff --git a/apps/nestjs-backend/src/features/attachments/attachments-storage.service.ts b/apps/nestjs-backend/src/features/attachments/attachments-storage.service.ts
@@ -3,7 +3,10 @@ import { PrismaService } from '@teable/db-main-prisma';
 import { UploadType } from '@teable/openapi';
 import { CacheService } from '../../cache/cache.service';
 import { IStorageConfig, StorageConfig } from '../../configs/storage';
+import { EventEmitterService } from '../../event-emitter/event-emitter.service';
+import { Events } from '../../event-emitter/events';
 import {
+  generateTableThumbnailPath,
   getTableThumbnailSize,
   getTableThumbnailToken,
 } from '../../utils/generate-table-thumbnail-path';
@@ -17,6 +20,7 @@ export class AttachmentsStorageService {
   constructor(
     private readonly cacheService: CacheService,
     private readonly prismaService: PrismaService,
+    private readonly eventEmitterService: EventEmitterService,
     @StorageConfig() private readonly storageConfig: IStorageConfig,
     @InjectStorageAdapter() private readonly storageAdapter: StorageAdapter
   ) {}
@@ -71,7 +75,9 @@ export class AttachmentsStorageService {
     let url = previewCache?.url;
     if (!url) {
       url = await this.storageAdapter.getPreviewUrl(bucket, path, expiresIn, respHeaders);
-
+      if (!url) {
+        throw new BadRequestException(`Invalid token: ${token}`);
+      }
       await this.cacheService.set(
         `attachment:preview:${token}`,
         {
@@ -84,38 +90,63 @@ export class AttachmentsStorageService {
     return url;
   }
 
-  async getTableAttachmentThumbnailUrl(smThumbnailPath?: string, lgThumbnailPath?: string) {
-    const smThumbnailUrl = smThumbnailPath
-      ? await this.getPreviewUrlByPath(
-          StorageAdapter.getBucket(UploadType.Table),
-          smThumbnailPath,
-          getTableThumbnailToken(smThumbnailPath)
-        )
-      : undefined;
-    const lgThumbnailUrl = lgThumbnailPath
-      ? await this.getPreviewUrlByPath(
-          StorageAdapter.getBucket(UploadType.Table),
-          lgThumbnailPath,
-          getTableThumbnailToken(lgThumbnailPath)
-        )
-      : undefined;
+  private async getTableThumbnailUrl(path: string, token: string) {
+    const previewCache = await this.cacheService.get(`attachment:preview:${token}`);
+    if (previewCache?.url) {
+      return previewCache.url;
+    }
+    const url = await this.storageAdapter.getPreviewUrl(
+      StorageAdapter.getBucket(UploadType.Table),
+      path,
+      second(this.storageConfig.urlExpireIn)
+    );
+    if (url) {
+      await this.cacheService.set(
+        `attachment:preview:${token}`,
+        {
+          url,
+          expiresIn: second(this.storageConfig.urlExpireIn),
+        },
+        second(this.storageConfig.urlExpireIn)
+      );
+    }
+    return url;
+  }
+
+  async getTableAttachmentThumbnailUrl(path: string) {
+    const { smThumbnailPath, lgThumbnailPath } = generateTableThumbnailPath(path);
+    const smThumbnailUrl = await this.getTableThumbnailUrl(
+      smThumbnailPath,
+      getTableThumbnailToken(smThumbnailPath)
+    );
+    const lgThumbnailUrl = await this.getTableThumbnailUrl(
+      lgThumbnailPath,
+      getTableThumbnailToken(lgThumbnailPath)
+    );
     return { smThumbnailUrl, lgThumbnailUrl };
   }
 
   async cutTableImage(bucket: string, path: string, width: number, height: number) {
     const { smThumbnail, lgThumbnail } = getTableThumbnailSize(width, height);
-    const cutSmThumbnailPath = await this.storageAdapter.cutImage(
+    const { smThumbnailPath, lgThumbnailPath } = generateTableThumbnailPath(path);
+    const cutSmThumbnailPath = await this.storageAdapter.cropImage(
       bucket,
       path,
       smThumbnail.width,
-      smThumbnail.height
+      smThumbnail.height,
+      smThumbnailPath
     );
-    const cutLgThumbnailPath = await this.storageAdapter.cutImage(
+    const cutLgThumbnailPath = await this.storageAdapter.cropImage(
       bucket,
       path,
       lgThumbnail.width,
-      lgThumbnail.height
+      lgThumbnail.height,
+      lgThumbnailPath
     );
+    this.eventEmitterService.emit(Events.CROP_IMAGE, {
+      bucket,
+      path,
+    });
     return {
       smThumbnailPath: cutSmThumbnailPath,
       lgThumbnailPath: cutLgThumbnailPath,

diff --git a/apps/nestjs-backend/src/features/attachments/plugins/adapter.ts b/apps/nestjs-backend/src/features/attachments/plugins/adapter.ts
@@ -83,7 +83,7 @@ export default abstract class StorageAdapter {
       // eslint-disable-next-line @typescript-eslint/no-explicit-any
       [key: string]: any;
     }
-  ): Promise<string>;
+  ): Promise<string | undefined>;
 
   /**
    * uploadFile with file path
@@ -119,7 +119,14 @@ export default abstract class StorageAdapter {
    * @param path path name
    * @param width width
    * @param height height
+   * @param newPath save as new path
    * @returns cut image url
    */
-  abstract cutImage(bucket: string, path: string, width: number, height: number): Promise<string>;
+  abstract cropImage(
+    bucket: string,
+    path: string,
+    width: number,
+    height: number,
+    newPath?: string
+  ): Promise<string>;
 }
diff --git a/apps/nestjs-backend/src/features/attachments/plugins/local.spec.ts b/apps/nestjs-backend/src/features/attachments/plugins/local.spec.ts
@@ -316,15 +316,15 @@ describe('LocalStorage', () => {
     });
   });
 
-  describe('getPreviewUrl', () => {
+  describe('getPreviewUrlInner', () => {
     it('should get preview URL', async () => {
       const mockBucket = 'mock-bucket';
       const mockPath = 'mock/file/path';
       const mockExpiresIn = 3600;
 
       vi.spyOn(storage.expireTokenEncryptor, 'encrypt').mockReturnValueOnce('mock-token');
 
-      const result = await storage.getPreviewUrl(
+      const result = await storage.getPreviewUrlInner(
         mockBucket,
         mockPath,
         mockExpiresIn,

diff --git a/apps/nestjs-backend/src/features/attachments/plugins/local.ts b/apps/nestjs-backend/src/features/attachments/plugins/local.ts
@@ -15,7 +15,6 @@ import { Encryptor } from '../../../utils/encryptor';
 import { second } from '../../../utils/second';
 import StorageAdapter from './adapter';
 import type { ILocalFileUpload, IObjectMeta, IPresignParams, IRespHeaders } from './types';
-import { generateCutImagePath } from './utils';
 
 interface ITokenEncryptor {
   expiresDate: number;
@@ -212,14 +211,25 @@ export class LocalStorage implements StorageAdapter {
     path: string,
     expiresIn: number = second(this.config.urlExpireIn),
     respHeaders?: IRespHeaders
-  ): Promise<string> {
+  ): Promise<string | undefined> {
+    if (!fse.existsSync(resolve(this.storageDir, bucket, path))) {
+      return undefined;
+    }
+    return this.getPreviewUrlInner(bucket, path, expiresIn, respHeaders);
+  }
+
+  async getPreviewUrlInner(
+    bucket: string,
+    path: string,
+    expiresIn: number,
+    respHeaders?: IRespHeaders
+  ) {
     const url = this.getUrl(bucket, path, {
       expiresDate: Math.floor(Date.now() / 1000) + expiresIn,
       respHeaders,
     });
     return this.baseConfig.storagePrefix + join('/', url);
   }
-
   verifyReadToken(token: string) {
     try {
       const { expiresDate, respHeaders } = this.expireTokenEncryptor.decrypt(token);
@@ -281,14 +291,15 @@ export class LocalStorage implements StorageAdapter {
     };
   }
 
-  async cutImage(bucket: string, path: string, width: number, height: number) {
-    const newPath = generateCutImagePath(path, width, height);
+  async cropImage(bucket: string, path: string, width: number, height: number, _newPath?: string) {
+    const newPath = _newPath || `${path}_${width}_${height}`;
     const resizedImagePath = resolve(this.storageDir, bucket, newPath);
     if (fse.existsSync(resizedImagePath)) {
       return newPath;
     }
+
     const imagePath = resolve(this.storageDir, bucket, path);
-    const image = sharp(imagePath);
+    const image = sharp(imagePath, { failOn: 'none', unlimited: true });
     const metadata = await image.metadata();
     if (!metadata.width || !metadata.height) {
       throw new BadRequestException('Invalid image');

diff --git a/apps/nestjs-backend/src/features/attachments/plugins/minio.ts b/apps/nestjs-backend/src/features/attachments/plugins/minio.ts
@@ -10,7 +10,6 @@ import { IStorageConfig, StorageConfig } from '../../../configs/storage';
 import { second } from '../../../utils/second';
 import StorageAdapter from './adapter';
 import type { IPresignParams, IPresignRes, IRespHeaders } from './types';
-import { generateCutImagePath } from './utils';
 
 @Injectable()
 export class MinioStorage implements StorageAdapter {
@@ -125,6 +124,9 @@ export class MinioStorage implements StorageAdapter {
     expiresIn: number = second(this.config.urlExpireIn),
     respHeaders?: IRespHeaders
   ) {
+    if (!(await this.fileExists(bucket, path))) {
+      return;
+    }
     const { 'Content-Disposition': contentDisposition, ...headers } = respHeaders ?? {};
     return this.minioClient.presignedGetObject(bucket, path, expiresIn, {
       ...headers,
@@ -178,8 +180,8 @@ export class MinioStorage implements StorageAdapter {
     }
   }
 
-  async cutImage(bucket: string, path: string, width: number, height: number) {
-    const newPath = generateCutImagePath(path, width, height);
+  async cropImage(bucket: string, path: string, width: number, height: number, _newPath?: string) {
+    const newPath = _newPath || `${path}_${width}_${height}`;
     const resizedImagePath = resolve(
       StorageAdapter.TEMPORARY_DIR,
       encodeURIComponent(join(bucket, newPath))

diff --git a/apps/nestjs-backend/src/features/attachments/plugins/s3.ts b/apps/nestjs-backend/src/features/attachments/plugins/s3.ts
@@ -18,7 +18,6 @@ import { IStorageConfig, StorageConfig } from '../../../configs/storage';
 import { second } from '../../../utils/second';
 import StorageAdapter from './adapter';
 import type { IPresignParams, IPresignRes, IObjectMeta, IRespHeaders } from './types';
-import { generateCutImagePath } from './utils';
 
 @Injectable()
 export class S3Storage implements StorageAdapter {
@@ -148,12 +147,15 @@ export class S3Storage implements StorageAdapter {
       height,
     };
   }
-  getPreviewUrl(
+  async getPreviewUrl(
     bucket: string,
     path: string,
     expiresIn: number = second(this.config.urlExpireIn),
     respHeaders?: IRespHeaders
-  ): Promise<string> {
+  ): Promise<string | undefined> {
+    if (!(await this.fileExists(bucket, path))) {
+      return;
+    }
     const command = new GetObjectCommand({
       Bucket: bucket,
       Key: path,
@@ -231,8 +233,8 @@ export class S3Storage implements StorageAdapter {
     }
   }
 
-  async cutImage(bucket: string, path: string, width: number, height: number) {
-    const newPath = generateCutImagePath(path, width, height);
+  async cropImage(bucket: string, path: string, width: number, height: number, _newPath?: string) {
+    const newPath = _newPath || `${path}_${width}_${height}`;
     const resizedImagePath = resolve(
       StorageAdapter.TEMPORARY_DIR,
       encodeURIComponent(join(bucket, newPath))
@@ -248,10 +250,9 @@ export class S3Storage implements StorageAdapter {
     if (!mimetype?.startsWith('image/')) {
       throw new BadRequestException('Invalid image');
     }
-    const metaReader = sharp();
+    const metaReader = sharp({ failOn: 'none', unlimited: true }).resize(width, height);
     const sharpReader = (stream as Readable).pipe(metaReader);
-    const resizedImage = sharpReader.resize(width, height);
-    await resizedImage.toFile(resizedImagePath);
+    await sharpReader.toFile(resizedImagePath);
     const upload = await this.uploadFileWidthPath(bucket, newPath, resizedImagePath, {
       'Content-Type': mimetype,
     });

diff --git a/apps/nestjs-backend/src/features/attachments/plugins/types.ts b/apps/nestjs-backend/src/features/attachments/plugins/types.ts
@@ -33,3 +33,8 @@ export type IRespHeaders = {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   [key: string]: any;
 };
+
+export enum ThumbnailSize {
+  SM = 'sm',
+  LG = 'lg',
+}
diff --git a/apps/nestjs-backend/src/features/attachments/plugins/utils.ts b/apps/nestjs-backend/src/features/attachments/plugins/utils.ts
@@ -2,6 +2,7 @@ import { join } from 'path';
 import { baseConfig } from '../../../configs/base.config';
 import { storageConfig } from '../../../configs/storage';
 import { LocalStorage } from './local';
+import type { ThumbnailSize } from './types';
 
 export const getFullStorageUrl = (bucket: string, path: string) => {
   const { storagePrefix } = baseConfig();
@@ -12,6 +13,6 @@ export const getFullStorageUrl = (bucket: string, path: string) => {
   return storagePrefix + join('/', bucket, path);
 };
 
-export const generateCutImagePath = (path: string, width: number, height: number) => {
-  return `${path}_${width}_${height}`;
+export const generateCropImagePath = (path: string, size: ThumbnailSize) => {
+  return `${path}_${size}`;
 };
diff --git a/apps/nestjs-backend/src/features/record/open-api/record-open-api.service.ts b/apps/nestjs-backend/src/features/record/open-api/record-open-api.service.ts
@@ -255,8 +255,8 @@ export class RecordOpenApiService {
           const attachmentCv = newCellValues[i] as IAttachmentCellValue;
           if (field.type === FieldType.Attachment && attachmentCv) {
             attachmentCv.forEach((attachmentItem, index) => {
-              const { mimetype, lgThumbnailPath, smThumbnailPath } = attachmentItem;
-              if (mimetype.startsWith('image/') && (!lgThumbnailPath || !smThumbnailPath)) {
+              const { mimetype } = attachmentItem;
+              if (mimetype.startsWith('image/')) {
                 collectionAttachmentThumbnails.push({
                   index: i,
                   key: fieldIdOrName,
@@ -275,22 +275,12 @@ export class RecordOpenApiService {
         if (!width || !height) {
           continue;
         }
-        const { smThumbnailPath, lgThumbnailPath } =
-          await this.attachmentsStorageService.cutTableImage(
-            StorageAdapter.getBucket(UploadType.Table),
-            path,
-            width,
-            height
-          );
-        attachmentItem.lgThumbnailPath = lgThumbnailPath;
-        attachmentItem.smThumbnailPath = smThumbnailPath;
-        const { smThumbnailUrl, lgThumbnailUrl } =
-          await this.attachmentsStorageService.getTableAttachmentThumbnailUrl(
-            smThumbnailPath,
-            lgThumbnailPath
-          );
-        attachmentItem.smThumbnailUrl = smThumbnailUrl;
-        attachmentItem.lgThumbnailUrl = lgThumbnailUrl;
+        this.attachmentsStorageService.cutTableImage(
+          StorageAdapter.getBucket(UploadType.Table),
+          path,
+          width,
+          height
+        );
       }
     }
     return records.map((record, i) => ({

diff --git a/apps/nestjs-backend/src/features/record/record.service.ts b/apps/nestjs-backend/src/features/record/record.service.ts
@@ -1045,7 +1045,7 @@ export class RecordService {
 
     return await Promise.all(
       cellValue.map(async (item) => {
-        const { path, mimetype, token, lgThumbnailPath, smThumbnailPath } = item;
+        const { path, mimetype, token } = item;
         const presignedUrl = await this.attachmentStorageService.getPreviewUrlByPath(
           StorageAdapter.getBucket(UploadType.Table),
           path,
@@ -1058,10 +1058,7 @@ export class RecordService {
         );
         return {
           ...item,
-          ...(await this.attachmentStorageService.getTableAttachmentThumbnailUrl(
-            smThumbnailPath,
-            lgThumbnailPath
-          )),
+          ...(await this.attachmentStorageService.getTableAttachmentThumbnailUrl(path)),
           presignedUrl,
         };
       })