refactor(s3): simplify s3 service

Author: ycdzj

src/attachments/attachments.service.ts

@@ -2,22 +2,19 @@ import {
   encodeFileName,
   getOriginalFileName,
 } from 'omniboxd/utils/encode-filename';
-import { Injectable, Logger, HttpStatus } from '@nestjs/common';
-import { AppException } from 'omniboxd/common/exceptions/app.exception';
-import { I18nService } from 'nestjs-i18n';
+import { Injectable, Logger } from '@nestjs/common';
 import { Response } from 'express';
-import { S3Service } from 'omniboxd/s3/s3.service';
+import { ObjectMeta, S3Service } from 'omniboxd/s3/s3.service';
 import { PermissionsService } from 'omniboxd/permissions/permissions.service';
 import { ResourcePermission } from 'omniboxd/permissions/resource-permission.enum';
-import { objectStreamResponse } from 'omniboxd/s3/utils';
 import { ResourceAttachmentsService } from 'omniboxd/resource-attachments/resource-attachments.service';
 import {
   UploadAttachmentsResponseDto,
   UploadedAttachmentDto,
 } from './dto/upload-attachments-response.dto';
-import { SharesService } from 'omniboxd/shares/shares.service';
 import { SharedResourcesService } from 'omniboxd/shared-resources/shared-resources.service';
 import { Share } from 'omniboxd/shares/entities/share.entity';
+import { Readable } from 'stream';
 
 @Injectable()
 export class AttachmentsService {
@@ -27,31 +24,53 @@ export class AttachmentsService {
     private readonly s3Service: S3Service,
     private readonly permissionsService: PermissionsService,
     private readonly resourceAttachmentsService: ResourceAttachmentsService,
-    private readonly sharesService: SharesService,
     private readonly sharedResourcesService: SharedResourcesService,
-    private readonly i18n: I18nService,
   ) {}
 
-  async checkPermission(
-    namespaceId: string,
-    resourceId: string,
-    userId: string,
-    permission: ResourcePermission = ResourcePermission.CAN_VIEW,
-  ) {
-    const hasPermission = await this.permissionsService.userHasPermission(
-      namespaceId,
-      resourceId,
-      userId,
-      permission,
-    );
-    if (!hasPermission) {
-      const message = this.i18n.t('auth.errors.notAuthorized');
-      throw new AppException(message, 'NOT_AUTHORIZED', HttpStatus.FORBIDDEN);
+  private s3Path(attachmentId: string): string {
+    return `attachments/${attachmentId}`;
+  }
+
+  private isMedia(mimetype?: string): boolean {
+    for (const type of ['image/', 'audio/']) {
+      if (mimetype?.startsWith(type)) {
+        return true;
+      }
     }
+    return false;
   }
 
-  s3Path(attachmentId: string): string {
-    return `attachments/${attachmentId}`;
+  private objectStreamResponse(
+    objectStream: Readable,
+    objectMeta: ObjectMeta,
+    httpResponse: Response,
+    cacheControl: boolean = true,
+    forceDownload: boolean = true,
+  ) {
+    const headers: Record<string, string> = {};
+    if (objectMeta.metadata?.filename) {
+      const disposition = forceDownload ? 'attachment' : 'inline';
+      headers['Content-Disposition'] =
+        `${disposition}; filename*=UTF-8''${encodeURIComponent(objectMeta.metadata.filename)}`;
+    }
+    if (objectMeta.contentType) {
+      headers['Content-Type'] = objectMeta.contentType;
+    }
+    if (objectMeta.contentLength) {
+      headers['Content-Length'] = objectMeta.contentLength.toString();
+    }
+    if (objectMeta.lastModified) {
+      headers['Last-Modified'] = objectMeta.lastModified.toUTCString();
+    }
+    if (cacheControl) {
+      headers['Cache-Control'] = 'public, max-age=31536000'; // 1 year
+    } else {
+      headers['Cache-Control'] = 'no-cache, no-store, must-revalidate';
+    }
+    for (const [key, value] of Object.entries(headers)) {
+      httpResponse.setHeader(key, value);
+    }
+    objectStream.pipe(httpResponse);
   }
 
   async uploadAttachment(
@@ -62,7 +81,7 @@ export class AttachmentsService {
     buffer: Buffer,
     mimetype: string,
   ) {
-    await this.checkPermission(
+    await this.permissionsService.userHasPermissionOrFail(
       namespaceId,
       resourceId,
       userId,
@@ -89,7 +108,7 @@ export class AttachmentsService {
     userId: string,
     files: Express.Multer.File[],
   ): Promise<UploadAttachmentsResponseDto> {
-    await this.checkPermission(
+    await this.permissionsService.userHasPermissionOrFail(
       namespaceId,
       resourceId,
       userId,
@@ -135,7 +154,7 @@ export class AttachmentsService {
     userId: string,
     httpResponse: Response,
   ) {
-    await this.checkPermission(
+    await this.permissionsService.userHasPermissionOrFail(
       namespaceId,
       resourceId,
       userId,
@@ -148,16 +167,11 @@ export class AttachmentsService {
       attachmentId,
     );
 
-    const objectResponse = await this.s3Service.get(
+    const { stream, meta } = await this.s3Service.getObject(
       this.s3Path(attachmentId),
     );
-
-    // Display media files inline, download other files as attachments
-    const forceDownload = !this.isMedia(objectResponse.mimetype);
-
-    return objectStreamResponse(objectResponse, httpResponse, {
-      forceDownload,
-    });
+    const forceDownload = !this.isMedia(meta.contentType);
+    this.objectStreamResponse(stream, meta, httpResponse, true, forceDownload);
   }
 
   async deleteAttachment(
@@ -166,7 +180,7 @@ export class AttachmentsService {
     attachmentId: string,
     userId: string,
   ) {
-    await this.checkPermission(
+    await this.permissionsService.userHasPermissionOrFail(
       namespaceId,
       resourceId,
       userId,
@@ -197,25 +211,10 @@ export class AttachmentsService {
       resourceId,
       attachmentId,
     );
-
-    const objectResponse = await this.s3Service.get(
+    const { stream, meta } = await this.s3Service.getObject(
       this.s3Path(attachmentId),
     );
-
-    // Display media files inline, download other files as attachments
-    const forceDownload = !this.isMedia(objectResponse.mimetype);
-
-    return objectStreamResponse(objectResponse, httpResponse, {
-      forceDownload,
-    });
-  }
-
-  isMedia(mimetype: string): boolean {
-    for (const type of ['image/', 'audio/']) {
-      if (mimetype.startsWith(type)) {
-        return true;
-      }
-    }
-    return false;
+    const forceDownload = !this.isMedia(meta.contentType);
+    this.objectStreamResponse(stream, meta, httpResponse, true, forceDownload);
   }
 }

src/namespace-resources/namespace-resources.service.ts

@@ -890,10 +890,10 @@ export class NamespaceResourcesService {
     }
     const artifactName = resource.id;
 
-    const fileStream = await this.s3Service.getObject(
+    const { stream } = await this.s3Service.getObject(
       this.s3Path(artifactName),
     );
-    return { fileStream, resource };
+    return { fileStream: stream, resource };
   }
 
   async getAllResourcesByUser(

src/permissions/permissions.service.ts

@@ -508,6 +508,24 @@ export class PermissionsService {
     return comparePermission(permission, requiredPermission) >= 0;
   }
 
+  async userHasPermissionOrFail(
+    namespaceId: string,
+    resourceId: string,
+    userId: string,
+    permission: ResourcePermission = ResourcePermission.CAN_VIEW,
+  ) {
+    const hasPermission = await this.userHasPermission(
+      namespaceId,
+      resourceId,
+      userId,
+      permission,
+    );
+    if (!hasPermission) {
+      const message = this.i18n.t('auth.errors.notAuthorized');
+      throw new AppException(message, 'NOT_AUTHORIZED', HttpStatus.FORBIDDEN);
+    }
+  }
+
   /**
    * Filter resources by permission.
    * For each non-root resource specified, it's required that all its parents are also specified.

src/s3/s3.service.ts

@@ -4,37 +4,28 @@ import {
   S3Client,
   PutObjectCommand,
   GetObjectCommand,
-  HeadObjectCommand,
   DeleteObjectCommand,
   HeadBucketCommand,
   CreateBucketCommand,
 } from '@aws-sdk/client-s3';
 import { Readable } from 'stream';
 import generateId from 'omniboxd/utils/generate-id';
 import {
-  decodeFileName,
   encodeFileName,
   getOriginalFileName,
 } from 'omniboxd/utils/encode-filename';
 
 export interface PutOptions {
   id?: string;
-  metadata?: Record<string, any>;
+  metadata?: Record<string, string>;
   folder?: string;
 }
 
-export interface ObjectInfo {
-  filename: string;
-  mimetype: string;
-  metadata: Record<string, any>;
-  stat: {
-    size?: number;
-    lastModified?: Date;
-  };
-}
-
-export interface GetResponse extends ObjectInfo {
-  stream: Readable;
+export interface ObjectMeta {
+  contentType?: string;
+  contentLength?: number;
+  metadata?: Record<string, string>;
+  lastModified?: Date;
 }
 
 @Injectable()
@@ -130,13 +121,23 @@ export class S3Service implements OnModuleInit {
     await this.s3Client.send(command);
   }
 
-  async getObject(key: string): Promise<Readable> {
+  async getObject(
+    key: string,
+  ): Promise<{ stream: Readable; meta: ObjectMeta }> {
     const command = new GetObjectCommand({
       Bucket: this.bucket,
       Key: key,
     });
     const response = await this.s3Client.send(command);
-    return response.Body as Readable;
+    return {
+      stream: response.Body as Readable,
+      meta: {
+        contentType: response.ContentType,
+        contentLength: response.ContentLength,
+        metadata: response.Metadata,
+        lastModified: response.LastModified,
+      },
+    };
   }
 
   async deleteObject(key: string) {
@@ -153,39 +154,12 @@ export class S3Service implements OnModuleInit {
     mimetype: string,
     options?: PutOptions,
   ): Promise<string> {
-    const { id = this.generateId(filename), metadata = {} } = options || {};
+    const { id = this.generateId(filename) } = options || {};
     const path: string = options?.folder ? `${options.folder}/${id}` : id;
     await this.putObject(path, buffer, mimetype, {
+      ...options?.metadata,
       filename: encodeFileName(filename),
-      metadata: JSON.stringify(metadata),
     });
     return id;
   }
-
-  async info(objectName: string): Promise<ObjectInfo> {
-    const command = new HeadObjectCommand({
-      Bucket: this.bucket,
-      Key: objectName,
-    });
-    const response = await this.s3Client.send(command);
-    const metadataString: string =
-      response.Metadata?.metadata_string || response.Metadata?.metadata || '{}';
-    const encodedFilename: string = response.Metadata?.filename || '';
-    const filename: string = decodeFileName(encodedFilename);
-    const mimetype: string = response.ContentType || 'application/octet-stream';
-    const metadata: Record<string, any> = JSON.parse(metadataString);
-    const stat = {
-      size: response.ContentLength,
-      lastModified: response.LastModified,
-    };
-    return { filename, mimetype, metadata, stat };
-  }
-
-  async get(objectName: string): Promise<GetResponse> {
-    const [stream, info] = await Promise.all([
-      this.getObject(objectName),
-      this.info(objectName),
-    ]);
-    return { stream, ...info } as GetResponse;
-  }
 }

src/s3/utils.ts

@@ -44,7 +44,7 @@ export class ChunkManagerService {
     const buffers: Buffer[] = [];
     for (let i = 0; i < totalChunks; i++) {
       const chunkPath = this.getChunkPath(taskId, i);
-      const stream = await this.s3Service.getObject(chunkPath);
+      const { stream } = await this.s3Service.getObject(chunkPath);
       buffers.push(await buffer(stream));
     }
     return Buffer.concat(buffers).toString('utf-8');

src/wizard/chunk-manager.service.ts

@@ -489,7 +489,7 @@ export class WizardService {
   }
 
   async getHtmlFromMinioGzipFile(path: string) {
-    const stream = await this.s3Service.getObject(path);
+    const { stream } = await this.s3Service.getObject(path);
     const gunzip = createGunzip();
     return new Promise<string>((resolve, reject) => {
       const chunks: Buffer[] = [];